PyCQA / pydocstyle

docstring style checker
http://pydocstyle.org
MIT License
1.11k stars 189 forks source link

[Bug] D417 not raised for missing parameter when docstring contains further sections #459

Open nioncode opened 4 years ago

nioncode commented 4 years ago

The following example does not raise a D417:

def foo(a, b):
    """Foobar.

    Args:
        a: test

    Raises:
        KeyError: description
    """
    pass

There is also no error raised when replacing Raises with Returns, i.e. D417 is broken for both.

Without the Raises section, everything works correctly, i.e.:

def foo(a, b):
    """Foobar.

    Args:
        a: test

    """
    pass

prints D417: Missing argument descriptions in the docstring (argument(s) b are missing descriptions in 'foo' docstring).

sambhav commented 4 years ago

Yeah - the issue is related to how we treat conventions as a bundle of error codes, instead of a first class entity. If we indeed take advantage of the fact that the user is specifying the convention type - we can avoid such ambigious cases. @Nurdok what are your thoughts on this? Also currently any google style docstring that has a returns section yields no errors for other sections since its detected as a numpy section.

Nurdok commented 4 years ago

@samj1912 - as I commented on #478, I think using the --convention flag to determine how to parse docstrings SGTM, but I'd like to see a more detailed proposal of how this would work.

adamjstewart commented 2 years ago

@samj1912 if I understand correctly, any docstring containing a section that is present in both Google-style and Numpy-style, such as:

will be interpreted as Numpy-style instead of Google-style, is that correct? These are obviously very common sections for docstrings, so this effectively means that auto-detection of Google-style docstrings doesn't currently work.

There are many possible fixes for this:

  1. Instead of checking for numpy section names, check for numpy-only section names
  2. Use the --convention flag to force a particular style
  3. Add a new flag to force a particular style and keep --convention as a bundle of error codes only

Do you have a particular preference for any of these? I think 1 is the quickest/easiest and most flexible since it would work most of the time, even if a single project mixes both Numpy and Google style in different methods. I'm also perfectly okay with 2 or 3 since my projects don't mix styles and I can easily specify which convention I use.

I'm happy to submit a PR to try to implement any of these options, although I may need pointers on which files contain the style autodetection code.

Mr-Pepe commented 2 years ago

@adamjstewart The file checker.py contains the following, starting from line 1062:

@check_for(Definition)
def check_docstring_sections(self, definition, docstring):
    """Check for docstring sections."""
    if not docstring:
        return

    lines = docstring.split("\n")
    if len(lines) < 2:
        return

    found_numpy = yield from self._check_numpy_sections(
        lines, definition, docstring
    )
    if not found_numpy:
        yield from self._check_google_sections(
            lines, definition, docstring
        )

The Google checks are skipped if anything is found in self._check_numpy_sections.

I think treating a docstring as a numpy docstring although I have explicitly configured pydocstyle to use the Google convention is a bug. Option 2 proposed by @adamjstewart sounds like the best way forward to me because it is less fragile then the first option and does not introduce the complexity of Option 3.

My proposal:

The first three steps would be a refactoring and the fourth step would fix the actual bug. Let me know if that sounds reasonable and I would volunteer to implement this.

RomainTT commented 1 year ago

Hi all, It seems this issue is not fixed yet. Any plans or implementation ideas about it ? Thank you.