DOC: Clarify DataFrame.combine_first and Series.combine_first #40279
Conversation
HeidiNeuhaeuser
commented
Mar 6, 2021
HeidiNeuhaeuser
commented
- closes #xxxx
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
There was a problem hiding this comment.
Thanks @HeidiNeuhaeuser , I like the example
I just have some minor suggestions
Also, it would be good to share the docstrings between the Series and DataFrame methods (see to_markdown, among others, for an example of how this is one), either here or in a follow-up pull request
pandas/core/series.py
Outdated
| Combine two Series objects by filling null values in one Series with | ||
| non-null values from the other Series. |
There was a problem hiding this comment.
per the corresponding line in frame.py, perhaps mention that the resulting index will be the union of the two?
There was a problem hiding this comment.
It was in the "Notes" section, I moved that for consistency.
pandas/core/series.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| other : Series | ||
| The value(s) to be combined with the `Series`. | ||
| The value(s) to be used for filling null values in `Series`. |
There was a problem hiding this comment.
perhaps just "The value(s) to be used for filling null values"?
|
Hello @HeidiNeuhaeuser! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2021-03-07 19:40:27 UTC |
|
thanks @HeidiNeuhaeuser if you'd like to make a shared doc-string as suggested by @MarcoGorelli pls open a new PR |
