PyCQA / docformatter

Formats docstrings to follow PEP 257
https://pypi.python.org/pypi/docformatter
MIT License
530 stars 62 forks source link

Invalid removal of valid lines between classes #156

Closed fmigneault closed 1 year ago

fmigneault commented 1 year ago

Using docformatter==1.5.1 (not a problem in 1.5.0), the tool check/fixes (according to options) the following snippet of code (i.e.: it proposes to remove the 2 lines between AcceptHeader and AcceptLanguageHeader classes).

I'm adding more classes in the below snippet in case more "context" might be important. The full code is here: https://github.com/crim-ca/weaver/blob/master/weaver/wps_restapi/swagger_definitions.py#L495-L520 There are a lot more similar class definitions in this file and across the code base, but docformatter keeps tripping over only these specific lines for some reason I cannot figure out.

class LastModifiedHeader(ExtendedSchemaNode):
    description = "Modification date and time of the contents."
    name = "Last-Modified"
    schema_type = String
    example = "Thu, 13 Jan 2022 12:37:19 GMT"

class AcceptHeader(ExtendedSchemaNode):
    # ok to use 'name' in this case because target 'key' in the mapping must
    # be that specific value but cannot have a field named with this format
    name = "Accept"
    schema_type = String
    missing = drop
    default = ContentType.APP_JSON  # defaults to JSON for easy use within browsers
-
-
class AcceptLanguageHeader(ExtendedSchemaNode):
    # ok to use 'name' in this case because target 'key' in the mapping must
    # be that specific value but cannot have a field named with this format
    name = "Accept-Language"
    schema_type = String
    missing = drop
    default = AcceptLanguage.EN_CA
    # FIXME: oneOf validator for supported languages (?)

class JsonHeader(ExtendedMappingSchema):
    content_type = ContentTypeHeader(example=ContentType.APP_JSON, default=ContentType.APP_JSON)
weibullguy commented 1 year ago

@fmigneault I can tell you that it's caused by the in-line comment. If you place the comment on the line before or after default = ContentType.APP_JSON, docformatter is OK with the two newlines. That said, I haven't figured out exactly why docformatter doesn't like in-line comments, but I'm on it!

fmigneault commented 1 year ago

@weibullguy

This is odd. There are other similar in-line comment on the last property of the class here: https://github.com/crim-ca/weaver/blob/master/weaver/wps_restapi/swagger_definitions.py#L1137 https://github.com/crim-ca/weaver/blob/master/weaver/wps_restapi/swagger_definitions.py#L1160 https://github.com/crim-ca/weaver/blob/master/weaver/wps_restapi/swagger_definitions.py#L2054

But these do not trigger lines to be removed between classes as in the original case shown. Thanks for working on it.

weibullguy commented 1 year ago

See also #165. The problem seems to be the result of using default as the variable name.

weibullguy commented 1 year ago

@fmigneault tag v1.6.1-rc1 should have the fix you need if you're interested in giving it a try before I officially release v1.6.1.

fmigneault commented 1 year ago

@weibullguy

The original issue seems fixed. However, there was the following new item. A spacing line unrelated to documentation is removed.

def test_wps3_process_step_io_data_or_href():
    """
    Validates that 'data' literal values and 'href' file references are both handled as input for workflow steps
    corresponding to a WPS-3 process.
    """
    # [... more items ...]

    def mock_wps_request(method, url, *_, **kwargs):
        nonlocal test_reached_parse_inputs
-
        method = method.upper()
        # [...]

(full example: https://github.com/crim-ca/weaver/blob/57555a56172539011721c9c7a4cacb9a41dca2ff/tests/processes/test_wps3_process.py#L14)

weibullguy commented 1 year ago

@fmigneault v1.6.1-rc2 should fix the new problem without, hopefully, introducing any more.

fmigneault commented 1 year ago

@weibullguy Looks good in my project.