-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: Fix doc for first_valid_index #55236
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 doc for first_valid_index #55236
Conversation
@@ -12468,7 +12468,7 @@ def first_valid_index(self) -> Hashable | None: | |||
|
|||
Notes | |||
----- | |||
If all elements are non-NA/null, returns None. |
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.
Could you just remove this Notes
section and demonstrate these points in the Examples
section below?
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.
Done. One thing I was unsure of was how to handle returning None
in the docs:
- I ran it through the docstring validation script
python scripts/validate_docstrings.py pandas.core.generic.NDFrame.first_valid_index
- It was throwing errors about the output not matching the expected output because I had originally specified:
>>> s = pd.Series()
>>> s.first_valid_index()
None
>>> s.last_valid_index()
None
I considered printing the outputs so that the outputs would match but didn't see print statements used in that way in other parts of the code so I did not keep them. Let me know if printing is the way to go here and I can make the change :)
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.
To clarify, option 1:
>>> s = pd.Series()
>>> s.first_valid_index()
>>> s.last_valid_index()
Option 2:
>>> s = pd.Series()
>>> print(s.first_valid_index())
None
>>> print(s.last_valid_index())
None
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.
Option 2 would be fine to make it explicit in the docs
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.
Added the print statements
Thanks @jfadia |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.