Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid removal of valid lines between classes #156

Closed
fmigneault opened this issue Jan 23, 2023 · 7 comments · Fixed by #171 or #175
Closed

Invalid removal of valid lines between classes #156

fmigneault opened this issue Jan 23, 2023 · 7 comments · Fixed by #171 or #175
Labels
C: convention Relates to docstring format convention P: bug PEP 257 violation or existing functionality that doesn't work as documented
Milestone

Comments

@fmigneault
Copy link

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)
@fmigneault fmigneault changed the title Invalid removal of valid lines be classes Invalid removal of valid lines between classes Jan 23, 2023
@github-actions github-actions bot added the fresh This is a new issue label Jan 23, 2023
@weibullguy weibullguy added P: bug PEP 257 violation or existing functionality that doesn't work as documented C: convention Relates to docstring format convention and removed fresh This is a new issue labels Jan 24, 2023
@weibullguy weibullguy added this to the v2.0.0 milestone Jan 24, 2023
@weibullguy
Copy link
Member

@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
Copy link
Author

@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
Copy link
Member

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

@weibullguy
Copy link
Member

@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
Copy link
Author

fmigneault commented Apr 13, 2023

@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
Copy link
Member

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

@fmigneault
Copy link
Author

@weibullguy
Looks good in my project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: convention Relates to docstring format convention P: bug PEP 257 violation or existing functionality that doesn't work as documented
Projects
None yet
2 participants