Skip to content

Conversation

TomAugspurger
Copy link
Contributor

Closes #6904

There's just a bit of extra index handling that needs to be done before moving on to generic.pct_change(). I had to adjust that to use the .div and .sub ops instead of / and - to work with panels.

I wasn't sure why axis wasn't included as an actual names keyword arg. generic just looks for it in **kwargs. I did the same in panel.

A related issue was the Panel.shift() has a different argument signature than generic.shift(). I can make those consistent and put in a deprecation warning in this issue or in a new one.

@@ -1130,11 +1130,20 @@ def shift(self, lags, freq=None, axis='major'):
if freq:
return self.tshift(lags, freq, axis=axis)

# TODO: why raise? axis=0 works, seems to be fine. need to chcek aligning
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback any idea why this isn't allowed? wp.shift(0) doesn't raise, and seems to produce the correct answer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

their is a perf issue currently because of the recent change of panel.shift to generic.shift

see #6826

I think wp.shift(0) is technically ok (though not intuitive)

prob should have a panel.shift that enforces kwargs (and doesn't have positional args at all)

you can use some of the machinery in reindex to detect the correct signatures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I'm going to hold off on touching Panel.shift() until #6826 is settled.

@TomAugspurger
Copy link
Contributor Author

I added a vbench, but maybe it isn't worth it since really all that's happing is a panel.shift() and a panel.div(). Should I take it out?

@jreback
Copy link
Contributor

jreback commented Apr 18, 2014

it's fine

@jreback
Copy link
Contributor

jreback commented Apr 18, 2014

instead of specifying the items axis use info_axis for ndim > 2

maybe this should also be explict for ndim > 2
iow you must specify an axis ?
not sure if we do that in other functions

@jreback jreback added this to the 0.14.0 milestone Apr 18, 2014
@jreback jreback added the Bug label Apr 18, 2014
@TomAugspurger
Copy link
Contributor Author

I changed the default axis to major to be more consistent with panel.shift(). Wasn't sure what you meant about making it explicit. Do you mean adding the axis as a real kwarg?

@jreback
Copy link
Contributor

jreback commented Apr 19, 2014

axis should default to stat_axis (which is 1 for a panel)

Had to change the implementation in generic.pct_change
to not pass `periods` as a keyword arg to generic.
Once the signitures of DataFrame.shift() and Panel.shift()
are aligned, this change can be reverted.
@TomAugspurger
Copy link
Contributor Author

I finally understand what you meant about the axes (I've never really understood what info_axis and stat_axis were before now). This let me move everything to generic. Should be good now. Haven't tested with Panel4D and up, but in principle there's no reason it shouldn't work.

@jreback
Copy link
Contributor

jreback commented Apr 21, 2014

gr8 - generic is good! makes maint easier

u can simply put in the Panel4D notimementrd list (as this is prob not intuitive), look in panelnd for a list of functions that it disables

@TomAugspurger
Copy link
Contributor Author

Actually this will already raise NotImplemented since shift is on the NotImplemented list, and that's used in pct_change.

@jreback
Copy link
Contributor

jreback commented Apr 21, 2014

ok gr8

TomAugspurger pushed a commit that referenced this pull request Apr 21, 2014
ENH: Implement Panel pct_change
@TomAugspurger TomAugspurger merged commit 2c83a97 into pandas-dev:master Apr 21, 2014
@jreback jreback mentioned this pull request Apr 22, 2014
@TomAugspurger TomAugspurger deleted the panel_shift branch April 5, 2017 02:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Bug Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Numeric Operations Arithmetic, Comparison, and Logical operations Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panel pct_change bug
2 participants