/pandas

BUG: axis kw not propogating on pct_change #11150

Closed
wants to merge 14 commits into
from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Numeric
Projects
None yet
Milestone
  0.17.1
5 participants
@Tux1 @sinhrks @jreback @rekcahpassyla @cfperez
@Tux1
Contributor

Tux1 commented Sep 19, 2015

axis option was not correctly set on fillna in pct_change
@Tux1 Tux1 Update generic.py 
axis option was not correctly set on fillna
af473cf
@sinhrks
Member

sinhrks commented Sep 19, 2015

Thanks. Can you add a test for this?

sinhrks added the Numeric label Sep 19, 2015

jreback changed the title from Update generic.py to BUG: axis kw not propogating on pct_change Sep 20, 2015

@jreback
Contributor

jreback commented Oct 5, 2015

@Tux1 pls add a test for this

Tux1 closed this Oct 5, 2015

Tux1 reopened this Oct 5, 2015

@Tux1
Contributor

Tux1 commented Oct 5, 2015

Sorry. It is my first commit.
I made a simple test that shows the issue and my commit fixed it.
Is it okay ?
Thanks
@jreback
Contributor

jreback commented Oct 5, 2015

pls see the contributing docs: http://pandas.pydata.org/pandas-docs/stable/contributing.html

tests / fix should be on a single pr.

jreback referenced this pull request Oct 5, 2015

Closed

test regarding fix about non propagating along axis on pct_change #11241

@Tux1
Contributor

Tux1 commented Oct 5, 2015

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.

@jreback jreback commented on an outdated diff Oct 9, 2015

pandas/tests/test_generic.py
@@ -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)
@jreback

jreback Oct 9, 2015

Contributor

add the issue number here
@jreback
Contributor

jreback commented Oct 9, 2015

pls add a whatsnew note for 0.17.1 (bug fix), use this PR number

jreback added this to the 0.17.1 milestone Oct 9, 2015

jreback added the Bug label Oct 9, 2015

@jreback jreback commented on an outdated diff Oct 9, 2015

pandas/tests/test_generic.py
@@ -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)
+ pnl.iat[1,0] = np.nan
+ pnl.iat[1,1] = np.nan
+
+ expected = pnl.ffill(axis=1).pct_change(axis=1, fill_method=None)
@jreback

jreback Oct 9, 2015

Contributor

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
@jreback
Contributor

jreback commented Oct 25, 2015

@Tux1 can you rebase / update
@Tux1
Contributor

Tux1 commented Oct 26, 2015

Sorry for several commits. I think that is okay now ? whatsnew edited, commit rebased

@jreback jreback commented on an outdated diff Oct 27, 2015

doc/source/whatsnew/v0.17.1.txt
@@ -96,7 +96,7 @@ Bug Fixes
-
+- Bug in ``DataFrame.pct_change()`` was not propagating axis on fillna method (:issue:`11150`)
@jreback

jreback Oct 27, 2015

Contributor

use double backticks around axis (and say axis keyword) and 'on .fillna() method'

@jreback jreback commented on an outdated diff Oct 27, 2015

pandas/tests/test_generic.py
@@ -1859,6 +1859,26 @@ def test_pipe_panel(self):
with tm.assertRaises(ValueError):
result = wp.pipe((f, 'y'), x=1, y=1)
+ 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)
@jreback

jreback Oct 27, 2015

Contributor

add the issue number as a comment here

@jreback jreback and 1 other commented on an outdated diff Oct 27, 2015

pandas/tests/test_generic.py
@@ -1859,6 +1859,26 @@ def test_pipe_panel(self):
with tm.assertRaises(ValueError):
result = wp.pipe((f, 'y'), x=1, y=1)
+ 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
+ pnl.iat[2,3] = 60
+
+ mask = pnl.isnull()
+
+ expected_axis0 = pnl.ffill(axis=0)/pnl.ffill(axis=0).shift(axis=0) - 1
+ expected_axis0[mask] = np.nan
+ result_axis0 = pnl.pct_change(axis=0, fill_method='pad')
+
+ self.assert_frame_equal(result_axis0, expected_axis0)
+
@jreback

jreback Oct 27, 2015

Contributor

move this to test_frame right after the other test_pct_change_* tests

@Tux1

Tux1 Oct 27, 2015

Contributor

what do you mean ? There is no test_frame and I made the only one test_pct_change_frame test

@jreback jreback commented on an outdated diff Oct 27, 2015

pandas/tests/test_generic.py
@@ -1859,6 +1859,26 @@ def test_pipe_panel(self):
with tm.assertRaises(ValueError):
result = wp.pipe((f, 'y'), x=1, y=1)
+ 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
+ pnl.iat[2,3] = 60
+
+ mask = pnl.isnull()
+
+ expected_axis0 = pnl.ffill(axis=0)/pnl.ffill(axis=0).shift(axis=0) - 1
@jreback

jreback Oct 27, 2015

Contributor

you can do this in a loop over both axes
@jreback
Contributor

jreback commented Oct 27, 2015

some comments, pls squash when finished
@jreback
Contributor

jreback commented Oct 28, 2015

pls rebase on master / squash, see docs here

@jreback jreback commented on an outdated diff Oct 28, 2015

pandas/tests/test_generic.py
@@ -1859,6 +1859,23 @@ def test_pipe_panel(self):
with tm.assertRaises(ValueError):
result = wp.pipe((f, 'y'), x=1, y=1)
+ # GH 11150
+ 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
@jreback

jreback Oct 28, 2015

Contributor

I mean that this function needs to move to tests\test_frame.py (and not in test_generic.py)
@Tux1
Contributor

Tux1 commented Nov 2, 2015

Sorry, my repo is a mess... I will be better next time

@jreback jreback added a commit that referenced this pull request Nov 13, 2015

@Tux1 @jreback Tux1 + jreback BUG: axis kw not propogating on pct_change #11150 334b076
@jreback
Contributor

jreback commented Nov 13, 2015

merged via 334b076

thanks!

jreback closed this Nov 13, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment