-
-
Notifications
You must be signed in to change notification settings - Fork 18.7k
DOC: Add docstring validations for "See Also" section #23143
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: Add docstring validations for "See Also" section #23143
Conversation
Hello @getschomp! Thanks for submitting the PR.
|
Codecov Report
@@ Coverage Diff @@
## master #23143 +/- ##
==========================================
+ Coverage 92.13% 92.13% +<.01%
==========================================
Files 170 170
Lines 51073 51077 +4
==========================================
+ Hits 47056 47060 +4
Misses 4017 4017
Continue to review full report at Codecov.
|
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.
Thanks for the PR. Looks quite good, just couple of comments.
""" | ||
Return the first 5 elements of the Series. | ||
|
||
see Also |
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.
I think the lower case is a typo here, right?
('BadSeeAlso', 'desc_no_period', | ||
('Missing period at end of description',)), | ||
('BadSeeAlso', 'desc_first_letter_lowercase', | ||
('should be capitalized',)), |
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.
Can you test that the element reported is the element with the problem? Meaning that Series.iloc
is ok, and in the error message we want to have that this is for Series.tail
.
scripts/validate_docstrings.py
Outdated
errs.append('Missing description for ' | ||
'See Also "{}" reference'.format(rel_name)) | ||
|
||
|
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.
This blank line is not valid according to PEP-8. If you take a look at the checks, you'll see that travis is red, and if you go to its log, you'll see that this is the reason.
Thanks, @datapythonista, made changes as requested. |
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. Thanks for the contribution @getschomp
@WillAyd can you take a look and merge on green if you're happy?
Thanks, @datapythonista, any word @WillAyd? |
Thanks @getschomp nice PR! |
git diff upstream/master -u -- "*.py" | flake8 --diff