BUG: rolling aggregate() with a list of functions along axis 1 raises ValueError #46132 #46892
Conversation
rolling aggregate() with a list of functions along axis 1 raise ValueError
jreback
changed the title
|
can you merge master and resolve conflicts |
|
@github-actions pre-commit |
pandas/core/window/rolling.py
Outdated
| _axis_modifed_flag = False | ||
| # modifying axis and transposing dataframe should not be needed | ||
| # once ReamplerWindow supports axis = 1 | ||
| if self.axis == 1: |
There was a problem hiding this comment.
why are we not using the _get_data_to_aggregate pattern here? this seems like making yet another way to do this.
There was a problem hiding this comment.
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.
_get_data_to_aggregate pattern was never implemented on rolling, just groupby.
Maybe instead @xr-chen, could you try to investigate assigning ResamplerWindowApply's self.axis attribute like GroupByApply fixes this instead?
self.axis = obj.obj._get_axis_number(kwargs.get("axis", 0))
There was a problem hiding this comment.
Thanks for your suggestion. I will have a try.
There was a problem hiding this comment.
I have tried to initialize ResamperWindowApply's self.axis like in GroupByApply, but that doesn't work. The agg_list_like method of Apply class behaves the same whatever the self.axis is, and it still calls obj._gotitem with subset as a Series and self.axis=1 which finally causes an error. @mroeschke
… assigning ResamplerWindowApply's attribute.
| self.axis = 0 | ||
|
|
||
| try: | ||
| result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() |
There was a problem hiding this comment.
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.
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.
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.
Ok, the agg method of ResamplerWindowApply hasn't been implemented, yet, and it will call the agg method of Apply class which can't recognize that we want to agg on axis=1 instead of axis=0. Even though I am not entirely familiar with pandas, I think passing the axis argument could not help.
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.