DOC: Fixing test and validate docstrings for section ordering #23245
Conversation
|
Hello @ryankarlos! Thanks for submitting the PR.
|
|
I have made additions to the validate_docstring.py and test_validate_docstrings.py - to check for the order of sections. However it is not easy to check this for all docstrings running scripts/validate_docstring.py as suggested in the issue - there is a lot of verbose with 'deprecated' warnings. Any suggestions ? |
|
Travis giving errors in Test validator, not sure why. |
ryankarlos
changed the title
|
Please merge master and can you not run I imagine this is because not all our docstrings are ordered correctly as the CI logs suggest:
We can omit running the ordering check on certain files for now and create an issue to follow up and fix. Depends what @datapythonista thinks :) |
There was a problem hiding this comment.
Added some feedback. Once this is working, it'd be nice to check which docstrings have the sections in the wrong order, and depending on how many we have, create issues to fix them. But that applies to all validations, and it can be taken care later on.
The validate_docstrings.py needs couple of changes until it can be used in the CI. Will take care of that later.
| @@ -146,6 +146,37 @@ def head1(self, n=5): | |||
| """ | |||
| return self.iloc[:n] | |||
|
|
|||
| def order(self): | |||
| """ | |||
| In this case, we are illustrating the correct ordering | |||
There was a problem hiding this comment.
If this is a example of good docstring , this summary should finish with a period and fit in one line. Tests should be failing because of it if this docstring is tested.
| case : bool, default True | ||
| Whether check should be done with case sensitivity. | ||
| na : object, default np.nan | ||
| Fill value for missing data. |
There was a problem hiding this comment.
The parameters should be in the signature, tests should also be failing
| Series | ||
| Original series with boolean values. | ||
|
|
||
| Examples |
There was a problem hiding this comment.
If this validates that order of sections is coreect, we should have all sectiona, see also and notes, and proba ly a extended summary too
| A nice greeting | ||
| """ | ||
|
|
||
| def wrong_section_order1(self): |
There was a problem hiding this comment.
I think to test all the sections we need to test every pair of contiguos section, parameters/return...
| continue | ||
| else: | ||
| section_index.append(self.raw_doc.index(section)) | ||
| return section_index |
There was a problem hiding this comment.
this implementation doesn't seem realiable, if the summary contains for example Returns... this will return the wrong indices, right?
Also, the name of the propertyis not very intuitive on what it does, and if something like this needs to be implemented, we should have tests for it.
I think we use numpydoc for parsing the docstring. May be there is something there that can be used?
There was a problem hiding this comment.
@datapythonista yup that’s true - I wanted to use numpydoc for parsing but wasn’t sure it would work for the summary and short summary sections as there is usually no title/header to parse the content.... trying to find a workaround
|
@ryankarlos do you have time to udate based on comments, and resolve the conflicts? |
|
@datapythonista I’ll try and work on this tonight as well as #23152 concurrently |
|
I've been researching on how to do this with numpydoc, and I ended up implementing this myself. Closing this, as superseded by #23607. |
git diff upstream/master -u -- "*.py" | flake8 --diff