pydocstyle icon indicating copy to clipboard operation
pydocstyle copied to clipboard

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

Open nioncode opened this issue 4 years ago • 5 comments

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).

nioncode avatar Mar 07 '20 15:03 nioncode

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.

samj1912 avatar May 05 '20 23:05 samj1912

@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.

Nurdok avatar May 28 '20 05:05 Nurdok

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

  • Returns
  • Raises
  • Yields
  • See Also
  • Notes
  • References
  • Examples
  • Attributes
  • Methods

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.

adamjstewart avatar Dec 29 '21 18:12 adamjstewart

@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:

  • Introduce a Convention class that holds the error codes to check for
  • Pass the convention as an argument to the check function (defaults to pep257)
  • Use the convention to initialize the ConventionChecker
  • Call the appropriate methods depending on the used convention

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.

Mr-Pepe avatar Apr 13 '22 20:04 Mr-Pepe

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

RomainTT avatar Jul 20 '23 12:07 RomainTT