DOC: Updates Index.reindex docstrings (#40328) #40879
Conversation
lnros
commented
Apr 11, 2021
•
lnros
commented
- closes DOC: pandas Index.reindex #40328: explains function parameters and gives basic example
- tests added / passed
- Ensure all linting tests pass, see here for how to run them
- whatsnew entry
|
Hello @lnros! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2021-05-25 11:23:31 UTC |
pandas/core/indexes/base.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| target : an iterable | ||
| method : {None, ‘backfill’/’bfill’, ‘pad’/’ffill’, ‘nearest’} |
There was a problem hiding this comment.
Is there a way to share parts with NDFrame.reindex?
There was a problem hiding this comment.
Maybe share parts with other functions in this same file, such as get_indexer? @phofl
There was a problem hiding this comment.
I think in this case you can better use , optional at the end, instead of having None in the list.
I don't think we ever use these forward ticks, can you check other docstrings to see if we use quotes at all or what.
There was a problem hiding this comment.
Thanks for the contribution @lnros, and sorry for the delay in reviewing. Great contribution, added some small comments about the standards we use, but looks great. Ping me when you address the comments, and the CI is green, and I'll have another look.
pandas/core/indexes/base.py
Outdated
|
|
||
| Parameters | ||
| ---------- | ||
| target : an iterable | ||
| method : {None, ‘backfill’/’bfill’, ‘pad’/’ffill’, ‘nearest’} |
There was a problem hiding this comment.
I think in this case you can better use , optional at the end, instead of having None in the list.
I don't think we ever use these forward ticks, can you check other docstrings to see if we use quotes at all or what.
pandas/core/indexes/base.py
Outdated
| - pad / ffill: Propagate last valid observation forward to next valid. | ||
| - backfill / bfill: Use next valid observation to fill gap. | ||
| - nearest: Use nearest valid observations to fill gap. | ||
| level : int or name |
There was a problem hiding this comment.
These are types, I guess it should be int or str?
pandas/core/indexes/base.py
Outdated
| level : int or name | ||
| Broadcast across a level, matching Index values on | ||
| the passed MultiIndex level. | ||
| limit : int, default None |
pandas/core/indexes/base.py
Outdated
| the passed MultiIndex level. | ||
| limit : int, default None | ||
| Maximum number of consecutive elements to forward or backward fill. | ||
| tolerance : optional |
There was a problem hiding this comment.
Type should be something like int or list-like of int, optional I think.
pandas/core/indexes/base.py
Outdated
| >>> index = ['a', 'b', 'c', 'd'] | ||
| >>> df = pd.DataFrame({"A": [1,2,3,4], | ||
| ... "B": [5,6,7,8], | ||
| ... "C": [9,10,11,12]}, |
There was a problem hiding this comment.
PEP-8 requires spaces ater commas, can you add them please, and make sure this is PEP-8 compliant.
| >>> target, indexer = df.index.reindex(target=['I', 'II', 'III', 'IV']) | ||
| >>> target, indexer | ||
| (Index(['I', 'II', 'III', 'IV'], dtype='object'), array([-1, -1, -1, -1])) | ||
| >>> target, indexer = df.index.reindex(target=['I', 'd', 'a', 'IV']) |
There was a problem hiding this comment.
Can you check the failure on the CI? These tests are failing, something is not right. You can test the code locally using the master version of pandas.
After you fix these things, can you check that the CI is green, so we can merge.
|
@lnros can you merge master to resolve conflicts |
|
@simonjayhawkins Done |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
closing as stale |
