BUG: axis kw not propogating on pct_change by Tux1 · Pull Request #11150 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation20 Commits14 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
axis option was not correctly set on fillna in pct_change
axis option was not correctly set on fillna
Thanks. Can you add a test for this?
Update generic.py BUG: axis kw not propogating on pct_change
@Tux1 pls add a test for this
Sorry. It is my first commit.
I made a simple test that shows the issue and my commit fixed it.
Is it okay ?
Thanks
Sorry... Moreover the test was wrong.
I have made another test which is passed with my patch. That is committed in a single branch, this one.
| @@ -1851,6 +1851,15 @@ def test_pipe_panel(self): |
|---|
| with tm.assertRaises(ValueError): |
| result = wp.pipe((f, 'y'), x=1, y=1) |
| def test_pct_change(self): |
| pnl = DataFrame([np.arange(0, 40, 10), np.arange(0, 40, 10), np.arange(0, 40, 10)]).astype(np.float64) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add the issue number here
pls add a whatsnew note for 0.17.1 (bug fix), use this PR number
| pnl.iat[1,0] = np.nan |
|---|
| pnl.iat[1,1] = np.nan |
| expected = pnl.ffill(axis=1).pct_change(axis=1, fill_method=None) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test with axis 0, 2 as well (if their are existing tests for that, pls move them here)
further this should be a bit more generic (or append _frame) to the test name as the generic.py tests should test Series,DataFrame,Panel
@Tux1 can you rebase / update
Tux1 and others added 5 commits
test regarding buf fix on pct_change propagation axis
revamping test_pct_change. Rename, multi axis and error fixed
Sorry for several commits. I think that is okay now ? whatsnew edited, commit rebased
| @@ -96,7 +96,7 @@ Bug Fixes |
|---|
| - Bug in ``DataFrame.pct_change()`` was not propagating axis on fillna method (:issue:`11150`) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use double backticks around axis (and say axis keyword) and 'on .fillna() method'
some comments, pls squash when finished
test_pct_change with loop over axis
backtick added
Add an optional argument to _NDFrameIndexer._align_series to indicate if the indexer is from a MultiIndex
pls rebase on master / squash, see docs here
| def test_pct_change_frame(self): |
|---|
| pnl = DataFrame([np.arange(0, 40, 10), np.arange(0, 40, 10), np.arange(0, 40, 10)]).astype(np.float64) |
| pnl.iat[1,0] = np.nan |
| pnl.iat[1,1] = np.nan |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean that this function needs to move to tests\test_frame.py (and not in test_generic.py)
Sorry, my repo is a mess... I will be better next time
jreback pushed a commit that referenced this pull request