-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC Fix inconsistencies #26301
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 Fix inconsistencies #26301
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26301 +/- ##
==========================================
- Coverage 91.98% 91.98% -0.01%
==========================================
Files 175 175
Lines 52374 52374
==========================================
- Hits 48178 48174 -4
- Misses 4196 4200 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26301 +/- ##
==========================================
- Coverage 92.04% 92.03% -0.01%
==========================================
Files 175 175
Lines 52301 52301
==========================================
- Hits 48141 48137 -4
- Misses 4160 4164 +4
Continue to review full report at Codecov.
|
@WillAyd @datapythonista doesn't the doc-string validator catch the invalid underlines? |
I don't see anything currently for underlines. Worth taking a look at; not sure how numpydoc would even parse that I am surprised some of these weren't raising GL06 for "unknown section". This is definitely part of the CI and should be raised when the section is called say "Example" instead of "Examples" |
I'm not sure why these weren't caught before but it looks like these fixes allowed the Docstring validation test to see that the docstring sections were in the wrong order. Maybe the ones that were out of order and had incorrect number of underlines were not be considered sections before, so they were not considered out of order. I'm also not sure where I would look at the Docstring validation to confirm this is what is happening... but should I go ahead and reorder them as part of this pr as well? |
Yea might as well reorder to get rid of any and all failures. FWIW the validator is located in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thank for taking care of all those @Scowley4
Not sure if there is something wrong with the validation of these. GL06
is being checked in the CI, and should report unknown headers like Example
, Return
.... But I think all the fixed ones are private, and I think we don't run the validation over them.
Having too many hyphens it does happen in public docstrings. In this case I think our validation gets the docstring already parsed from numpydoc, and I'm not sure if numpydoc is happy when there are more or less hyphens.
@Scowley4 do you want to do some research, and open an issue proposing what you think it's best?
Just saw that the CI is red, the problem is that the fixed sections are now detected, and they are in the wrong position with respect to the other sections. See:
So, looks like numpydoc is "ignoring" (not considering header sections) the ones with the wrong hyphens, which I think makes sense. I don't think we can detect wrong section names with wrong hyphens automatically, but we could add a validation that looks for the correct section names in the docstring, and checks if the section exists (if the section has been detected by numpydoc). I think if we check that the name of the section exists in the docstring in a single line, we shouldn't have false positives. For example, we can have in the notes @Scowley4 if you want to work on it, can you open an issue please? Otherwise I'll create it myself so someone else can work on it. |
@WillAyd Thanks for pointing me to the testing code. @datapythonista Totally agree with the "ignoring section header" thing and have opened an issue about it that I would like to work on. #26307 |
…ley4/pandas into scowley4-fixInconsistencies
Changes look good, but CI is red. Not sure what's the problem, may be you just need to merge master, the error seems unrelated to the docstring changes. |
@datapythonista Was trying to figure out what the error was. Wondering if I accidentally started down a rabbit hole (One fix exposes more problems and more and more...). Can you be a little more explicit about what you suggest I do? Did you mean for me to merge into my master branch? |
in you branch just do this will let in your branch your changes, but applied to the last version of pandas, not the version when you created the branch |
Same error. Maybe now that the sections are actually sections and in the correct order, there is something wrong inside one and that is throwing the error. I haven't found the fix yet, though... |
As far as I can tell, the only place Lines 206 to 211 in 2bbc0c2
|
I guess the above is just in In the full repository: Line 501 in 2bbc0c2
https://github.com/pandas-dev/pandas/blob/master/doc/source/reference/frame.rst And two places in It looks like this method was removed recently: #25047 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@datapythonista @WillAyd : if you'd like to double check before merging
@Scowley4 just had another PR go in which conflicts here - could you fix that up? Otherwise lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ping on green
thanks @Scowley4, nice clean up |
Fix a few inconsistencies in the docstrings.
Not sure how pandas community feels about these kinds of fixes (and this is my first PR to pandas). I just notice things that are different from how they occur most of the time and decided to fix them up.
Examples
Add 's' to headers
to
Correct number of hyphens under headers
to
Rewrite testcase in standard format
to