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

DOC: docstring validation script improvements #20298

Open
9 of 19 tasks
jorisvandenbossche opened this issue Mar 12, 2018 · 16 comments
Open
9 of 19 tasks

DOC: docstring validation script improvements #20298

jorisvandenbossche opened this issue Mar 12, 2018 · 16 comments
Labels
Docs Master Tracker High level tracker for similar issues Needs Discussion Requires discussion from core team before further action

Comments

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Mar 12, 2018

Based on the experience from the sprint, some known issues:

cc @TomAugspurger @datapythonista @WillAyd others that you noticed?

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Mar 12, 2018

There were some like

param : {'a', 'b'}
    - a : do this
    - b : do that

that complained about the explanation not starting with a capital letter. Maybe we're OK with that to force a general explanation before going into individual params.

Tangentially related: whitespace normalization for doctests (#20284)

@TomAugspurger
Copy link
Contributor

Under the see also, we could check for and error if they have pandas.DataFrame.apply instead of DataFrame.apply.

It'd be nice to somehow know if the target is valid (a decent amount of DataFrame.groupby.head-like references, but perhaps that's expensive to check?

@jorisvandenbossche
Copy link
Member Author

I think we can easily check if the first character is a - (sometimes adding such a first sentence can be superfluous, and you then have to leave a blank line)

Tangentially related: whitespace normalization for doctests (#20284)

We need to do this for pytest, but it is already handled separately in the validation_script

@datapythonista
Copy link
Member

If I'm not wrong, the validation wasn't checking much on the Returns section, at least I've reviewed many PRs without description, or with name+type...

CCing @kynan and @mariocj89, if I'm not wrong they did some work on the kwargs problem during the sprint.

@datapythonista
Copy link
Member

Also, I think it could be good to warn if the user is importing pandas or numpy in the examples, or if uses string, boolean, integer, or list-like in the type descriptions.

I can take care of this ticket if nobody is working on it already, I've got couple of fixes to the generation of the csv that I didn't send it yet.

@jorisvandenbossche
Copy link
Member Author

Yes, certainly go ahead with doing a PR

@WillAyd
Copy link
Member

WillAyd commented Mar 12, 2018

@datapythonista if it helps I started on a test for the validation script in #20061 - I can put some time into wrapping that up today so we can be sure we don't fix one thing and break another with this going forward

@TomAugspurger
Copy link
Contributor

The section order should be validated. I believe there's a numpydoc issue about making the order required.

@TomAugspurger
Copy link
Contributor

Would be nice to extract the code examples and pipe them through flake8 if possible.

@jorisvandenbossche
Copy link
Member Author

Would be nice to extract the code examples and pipe them through flake8 if possible.

Yes, that would be a nice one!

@TomAugspurger
Copy link
Contributor

TomAugspurger commented Mar 12, 2018 via email

@datapythonista
Copy link
Member

@WillAyd that sounds. I've already got some changes to the script, but it's more in the validate_all part, which I don't think you'll touch.

I love both ideas @TomAugspurger, validating both things will be very useful.

@jorisvandenbossche
Copy link
Member Author

Other point originally reported in #20318: if the docstring contains a newline, it gives problems in the validation script.

@WillAyd
Copy link
Member

WillAyd commented Aug 18, 2018

Edited to add a bullet for the discussion in #20061 (comment)

@kynan
Copy link
Contributor

kynan commented Feb 3, 2019

Some minor improvements in #25122

@ChiefMilesEdgeworth
Copy link
Contributor

A couple more issues that need to be fixed:

  • Allow for *args, **kwargs on one line with a description instead of having to split it each time
  • Change signature_parameters so that inspect.signature is used instead of inspect.getfullargspec (I talked about this a bit in Fix PR02 issues in docstrings #27976)

This will solve a lot of the false positives for PR01 and PR02 errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Master Tracker High level tracker for similar issues Needs Discussion Requires discussion from core team before further action
Projects
None yet
Development

No branches or pull requests

6 participants