CLN: Unify Window._apply_window and Rolling._apply functions #27403
Conversation
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
it should be possible to make this very generic
as well already support passing functions in _applly for Rolling
can u see how much code can be consilidated here
|
@jreback ok kept original function unchanged while calling it in separate functions. need to pass str functions in |
|
in the spirit of #27409, any objections to renaming to |
|
In favour of renaming it to |
ihsansecer
changed the title
ihsansecer
changed the title
|
It seems some network tests have failed |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
looks really good, some comments.
| window : int/array, default to _get_window() | ||
| center : bool, default to self.center | ||
| check_minp : function, default to _use_window | ||
There was a problem hiding this comment.
can you add what kwargs is here
There was a problem hiding this comment.
should we modify kwargs (or copy of it) to remove window function (passed to _get_window) kwargs first. window rolling functions (passed to _get_roll_func) don't take kwargs but when they do it will be a problem
| name : str, optional | ||
| name of this function | ||
| window : int/array, default to _get_window() | ||
| window : int/str, default to _get_window() |
There was a problem hiding this comment.
didn't see any passing array to _apply. This seems to be an offset or int same as in rolling function
pandas/core/window.py
Outdated
| index_as_array = self._get_index() | ||
|
|
||
| results = [] | ||
| exclude = [] |
There was a problem hiding this comment.
pandas/core/window.py:426: error: Need type annotation for 'exclude'
not sure what confuses mypy to require type for exclude (this is the reason of ci fail)
|
Would this be a good opportunity to include a fix for issue #26462 ? Currently it looks like the |
|
@ihsansecer pls merge master and update when you can, ping on green. |
@Connossor we try to keep 1 PR to 1 thing; this is a refactor only. |
pandas/core/window.py
Outdated
| center: Optional[bool] = None, | ||
| check_minp: Optional[Callable] = None, | ||
| **kwargs | ||
| ) -> FrameOrSeries: |
There was a problem hiding this comment.
removed because mypy was complaining
| def _get_window(self, other=None): | ||
| def _get_window(self, other=None, **kwargs) -> int: | ||
| """ | ||
| Returns window lenght |
2 participants
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffroll_windowfunction is split into two to enable implementing other weighted functions e.g. var, std (ref #26597)