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

False positive for RST213 and RST210 from *arg and **kwargs #18

Open
sobolevn opened this issue Nov 18, 2019 · 14 comments
Open

False positive for RST213 and RST210 from *arg and **kwargs #18

sobolevn opened this issue Nov 18, 2019 · 14 comments

Comments

@sobolevn
Copy link

This code produces two violations:

def include(*args: str, **kwargs) -> None:
    """
    Used for including Django project settings from multiple files.

    Args:
        *args: File paths (``glob`` - compatible wildcards can be used).
        **kwargs: Settings context: ``scope=globals()`` or ``None``.

    """

Output:

» flake8 .

./split_settings/tools.py

  65:1     RST299 Inline emphasis start-string without end-string.
  Args:
  ^

  65:1     RST210 Inline strong start-string without end-string.
  Args:
  ^

That's because flake8-rst-docstrings is unhappy about *args and **kwargs in the docs. But, napoleon and google explicitly says to write them like so, proof: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google

def module_level_function(param1, param2=None, *args, **kwargs):
    """This is an example of a module level function.

    ...

    Args:
        param1 (int): The first parameter.
        param2 (:obj:`str`, optional): The second parameter. Defaults to None.
            Second line of description should be indented.
        *args: Variable length argument list.
        **kwargs: Arbitrary keyword arguments.
    
    ...
    """

So, I guess that these two rules should respect this case.

@peterjc
Copy link
Owner

peterjc commented Nov 18, 2019

That does seem to be a conflict - https://www.sphinx-doc.org/en/master/usage/extensions/example_numpy.html#example-numpy and https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html agree, but to me they look malformed.

Perhaps the extension special cases it? If so, we could do the same in RST210 and whatever unique code I should give for strong (since RST299 is a place holder).

@sobolevn
Copy link
Author

Sorry, I didn't quite get the "special cases" part. What do you suggest?

@peterjc
Copy link
Owner

peterjc commented Nov 18, 2019

The following is clearly wrong in RST:

*emphasis
**strong

Yet the extensions seem to recommend this:

*args
**kwargs

Perhaps the extension does something special with these two words?

Also, I see I am missing a bunch of related messages like:

  • Inline literal start-string without end-string.
  • Inline interpreted text or phrase reference start-string without end-string.

See:

https://sourceforge.net/p/docutils/code/HEAD/tree/trunk/docutils/docutils/parsers/rst/states.py

@sobolevn
Copy link
Author

Oh, I see. The notation of *args and **kwargs is incorrect rst syntax.
I have also seen \*args and \*\*kwargs here: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google

@peterjc
Copy link
Owner

peterjc commented Nov 18, 2019

Yes, the following are fine - escaping with slashes, or wrapping with back-ticks:

If \*args or \*\*kwargs are accepted,
they should be listed as ``*args`` and ``**kwargs``.

Perhaps it is worth opening an issue with the docstring extension to double check their intent.

peterjc added a commit that referenced this issue Nov 18, 2019
These are all examples fitting the pattern:

'Inline %s start-string without end-string.'

Raised in docutils/parsers/rst/states.py

Noted in passing on GitHub issue #18.
@peterjc
Copy link
Owner

peterjc commented Nov 18, 2019

I've released v0.0.12, so now instead of RST299 (ending 99 indicating a previously unseen error), you should get RST213.

@peterjc peterjc changed the title False positive for RST299 and RST210 False positive for RST213 and RST210 from *arg and **kwargs Dec 27, 2019
@peterjc
Copy link
Owner

peterjc commented Dec 27, 2019

I've recently tried the plugin on two large projects with lots of these false positives:

rigetti/pyquil#1141
astropy/astropy#9820

Do you think it should ignore *args and **kwargs by default?

Or, should there be configurable lists of variable names to ignore (since not all projects will use these exact conventions)?

@sobolevn
Copy link
Author

sobolevn commented Dec 28, 2019

Yes, args and kwargs can probably be exceptional.

@peterjc
Copy link
Owner

peterjc commented Apr 2, 2020

I don't see an easy way to fix this (a docutils expert might), but have a hack on PR #27 which might be worth testing?

@knoppo
Copy link

knoppo commented Jul 20, 2020

Hey, I stumbled over this, too and I think its worth mentioning that sphinx.ext.napoleon seems to auto-escape asterisks at the beginning of each argument. See https://github.com/sphinx-doc/sphinx/blob/3.x/sphinx/ext/napoleon/docstring.py#L326

Oh, I see. The notation of *args and **kwargs is incorrect rst syntax.
I have also seen \*args and \*\*kwargs here: https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google

You can actually find both notations in there. The asterisks are escaped everywhere except in the Args: section.

Considering this, I think it would be perfectly fine to treat them as special cases just like the napoleon extension does.

@peterjc
Copy link
Owner

peterjc commented Jul 20, 2020

Thanks for the pointer to the napoleon special casing code - I agree, it would be practical to follow their example.

@sobolevn
Copy link
Author

@peterjc
Copy link
Owner

peterjc commented Nov 11, 2020

I did update the README to suggest the following configuration if using the Google style:

[flake8]
extend-ignore =
    # Google Python style is not RST until after processed by Napoleon
    # See https://github.com/peterjc/flake8-rst-docstrings/issues/17
    RST201,RST203,RST301,

This is still not ideal from a usability point of view. Actually applying the napoleon transformation does seem the like best approach, but it adds complexity.

@argunv
Copy link

argunv commented Dec 22, 2023

I added * in the end of *args ... *. It helps.

peterjc added a commit that referenced this issue Dec 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants