-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
BUG: rolling aggregate() with a list of functions along axis 1 raises ValueError #46132 #46892
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 10 commits
740adf8
7a9d909
d7c9851
2d11a3b
ca3e961
a2ae8be
0782b2e
f716944
32afa43
fecb9ea
fa2e1cb
08fed94
e34d1a5
45185a9
8e21d87
7a35eb2
2a933d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -658,9 +658,22 @@ def _numba_apply( | |
return self._resolve_output(out, obj) | ||
|
||
def aggregate(self, func, *args, **kwargs): | ||
result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() | ||
# GH46132 | ||
_axis_modifed_flag = False | ||
if self.axis == 1: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we not using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you mean to write another function or use some methods in some python class to get the data ready for aggregation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Maybe instead @xr-chen, could you try to investigate assigning
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your suggestion. I will have a try. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tried to initialize |
||
self.obj = self.obj.T | ||
_axis_modifed_flag = True | ||
self.axis = 0 | ||
try: | ||
result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we just add an axis argument here and thread it thru. this state change is not at all obvious. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just would like to make sure if you want me to make this comment more clear? And I notice that in PR #47078, it says rolling window doesn't support axis = 1 anymore and that caused this change didn't pass the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i am not sure what you mean. I don't really like this approach because its hiding the axis state. What I am suggesting is we actually fix this by passing thru the axis argument to do this properly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, the |
||
finally: | ||
if _axis_modifed_flag: | ||
self.axis = 1 | ||
self.obj = self.obj.T | ||
if result is None: | ||
return self.apply(func, raw=False, args=args, kwargs=kwargs) | ||
if _axis_modifed_flag: | ||
result = result.T | ||
return result | ||
|
||
agg = aggregate | ||
|
Uh oh!
There was an error while loading. Please reload this page.