Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Fix indentation error while parsing class methods #441

Merged
merged 2 commits into from Jul 8, 2020

Conversation

samj1912
Copy link
Member

@samj1912 samj1912 commented Dec 15, 2019

In order to determine the missing arguments, we currently
use ast.parse to parse partial source code. This parsing
might lead to syntax errors. We catch the syntax error and
make the parsing more resilient to errors in the source code.

Fixes #437

Thanks for submitting a PR!

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable.
    If you've added an error code or changed an error code behavior,
    you should probably add or change a test case file under tests/test_cases/ and add
    it to the list under tests/test_definitions.py.
    If you've added or changed a command line option,
    you should probably add or change a test in tests/test_integration.py.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

Please don't get discouraged as it may take a while to get a review.

@samj1912
Copy link
Member Author

@Nurdok review please :)

@samj1912 samj1912 force-pushed the indent branch 2 times, most recently from 5c4a69b to aff6360 Compare January 10, 2020 22:01
@samj1912
Copy link
Member Author

@Nurdok bump?

@samj1912
Copy link
Member Author

@Nurdok bump

1 similar comment
@samj1912
Copy link
Member Author

@Nurdok bump

@tacaswell
Copy link

I don't think the test actually tests the issue in the original issue.

Because we need to put the full parameter specification on one line to make sphinx happy we use line continuation to wrap it, but if you indent the continuation, you end up with a whole bunch of spaces in the line.

I also do not agree that this is "incorrect" indentation for the same reason.

@samj1912
Copy link
Member Author

@Nurdok bump

Nurdok
Nurdok previously approved these changes May 5, 2020
src/pydocstyle/checker.py Outdated Show resolved Hide resolved
@samj1912
Copy link
Member Author

samj1912 commented May 5, 2020

I don't think the test actually tests the issue in the original issue.

Because we need to put the full parameter specification on one line to make sphinx happy we use line continuation to wrap it, but if you indent the continuation, you end up with a whole bunch of spaces in the line.

I also do not agree that this is "incorrect" indentation for the same reason.

The issue was caused because of "incorrectly" indented conent like #437 (comment). The parameters case you mention is a subset and is covered by this.

src/tests/test_cases/sections.py Outdated Show resolved Hide resolved
@tacaswell
Copy link

But the original issue (#437 (comment)) is correct. While I see why from a pure-parsing point of view these maybe the same, from a semantic point of view they are very different as with the \ line continuation the second line should not be "indented".

Can you please add a test of a "correctly" formatted docstring that uses line continuation as well?

@samj1912
Copy link
Member Author

samj1912 commented May 8, 2020

But the original issue (#437 (comment)) is correct. While I see why from a pure-parsing point of view these maybe the same, from a semantic point of view they are very different as with the \ line continuation the second line should not be "indented".

Can you please add a test of a "correctly" formatted docstring that uses line continuation as well?

Added a test. It works as expected.

@samj1912
Copy link
Member Author

samj1912 commented May 8, 2020

@Nurdok all comments should be addressed now. :)

In order to determine the missing arguments, we currently
use ast.parse to parse partial source code. This parsing
might lead to syntax errors. We catch the syntax error and
make the parsing more resilient to errors in the source code.

Fixes PyCQA#437
@samj1912
Copy link
Member Author

@Nurdok bump

Nurdok
Nurdok previously approved these changes May 28, 2020
@tacaswell
Copy link

Where we actually have to use line continuation in practice is in the type specification not in the body of the parameter description. A real life example: https://github.com/matplotlib/matplotlib/blob/5445d77460f71d16dd3e1b01c4c116a741baf6b9/lib/matplotlib/axes/_axes.py#L4392-L4402

class A:
    def standin(...):
        """
        ...

        Parameters
        ----------

        ...


        edgecolors : {'face', 'none', *None*} or color or sequence of color, \
default: :rc:`scatter.edgecolors`
            The edge color of the marker. Possible values:
            - 'face': The edge color will always be the same as the face color.
            - 'none': No patch boundary will be drawn.
            - A color or sequence of colors.
            For non-filled markers, the *edgecolors* kwarg is ignored and
            forced to 'face' internally.

        ...

        """
        ...

@samj1912
Copy link
Member Author

@tacaswell, I agree but this is unrelated to the original bug that this PR was made to solve. This has been a long standing issue long before #437 was reported. @anntzer has a fix for the line continuation bug at #472

Please see our conversation there for more details. Once this PR is merged, we will merge @anntzer 's PR with this fix.

@samj1912
Copy link
Member Author

samj1912 commented Jul 5, 2020

@Nurdok I can't merge because the release notes merge conflict dismissed your review. Can you approve again please?

@samj1912 samj1912 merged commit dd5ab9e into PyCQA:master Jul 8, 2020
@samj1912 samj1912 deleted the indent branch July 8, 2020 08:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IndentationError on line continuations for numpydoc
4 participants