DEPR/CLN: Remove freq parameters from df.rolling/expanding/ewm #18601
Conversation
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
0d5ae5d to
77b0b36
Compare
doc/source/whatsnew/v0.22.0.txt
Outdated
| @@ -133,6 +133,8 @@ Removal of prior version deprecations/changes | |||
| - ``pd.tseries.util.pivot_annual`` has been removed (deprecated since v0.19). Use ``pivot_table`` instead (:issue:`18370`) | |||
| - ``pd.tseries.util.isleapyear`` has been removed (deprecated since v0.19). Use ``.is_leap_year`` property in Datetime-likes instead (:issue:`18370`) | |||
| - ``pd.ordered_merge`` has been removed (deprecated since v0.19). Use ``pd.merge_ordered`` instead (:issue:`18459`) | |||
| - The ``freq`` parameter has been removed from the ``rolling``/``expanding``/``ewm`` methods of DataFrame | |||
| and Series. Instead, resample before calling the methods. (:issue:xxx) | |||
There was a problem hiding this comment.
Reference the original PR where it was deprecated.
|
@topper-123 : Thanks for this! Just need to patch a |
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
77b0b36 to
010cf95
Compare
Codecov Report
@@ Coverage Diff @@
## master #18601 +/- ##
==========================================
- Coverage 91.46% 91.44% -0.03%
==========================================
Files 157 157
Lines 51439 51426 -13
==========================================
- Hits 47051 47028 -23
- Misses 4388 4398 +10
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18601 +/- ##
==========================================
+ Coverage 91.58% 91.58% +<.01%
==========================================
Files 153 153
Lines 51250 51237 -13
==========================================
- Hits 46935 46924 -11
+ Misses 4315 4313 -2
Continue to review full report at Codecov.
|
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
2 times, most recently
from
03c04fe to
b81fc11
Compare
|
all green. The travis error seems unrelated to this PR. |
pandas/core/window.py
Outdated
| frequency by resampling the data. This is done with the default parameters | ||
| of :meth:`~pandas.Series.resample` (i.e. using the `mean`). | ||
| See Also | ||
| --------- |
There was a problem hiding this comment.
I think these need to be the same length as the text (the underlines)
There was a problem hiding this comment.
Actually, it has to be the same length or longer. But yeah, it's probably better style to keep same length. I'll change it.
pandas/stats/moments.py
Outdated
| @@ -208,6 +208,7 @@ def ensure_compat(dispatch, name, arg, func_kw=None, *args, **kwargs): | |||
| if value is not None: | |||
| kwds[k] = value | |||
|
|
|||
| kwargs.pop('freq', None) # freq removed in 0.22 | |||
There was a problem hiding this comment.
this shouldn't be necessary
There was a problem hiding this comment.
A bunch of tests in pandas\tests\test_window.py fail without this.
As I'll clean up that module after finishing up how (incl. all reference to freq for pd.stats.*), I suggest leaving it as it is. It not worth it to make the tests pass without this for a module that will be deleted shortly.
There was a problem hiding this comment.
that's fine, just add a TODO then
There was a problem hiding this comment.
Well, the whole moments.py module will be deleted, incl. this small change....
But ok, I'ved added it.
There was a problem hiding this comment.
I understand, and until that time, pls add a TODO
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
2 times, most recently
from
882d2c1 to
05c9006
Compare
|
All green. |
|
lint error & needs rebase |
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
05c9006 to
96899d7
Compare
|
green, I think. Wrt. linting, I don't get any warning locally. There is a warning that pops up, but that is already in master, so unrelated to my PR (unless I've misunderstood something). Next I will submit a PR on |
|
linting is an issue on this PR you can show it locally with |
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
96899d7 to
6527d76
Compare
|
Thanks, I've seen it on travis now. Can't get |
|
yeah sorry it’s a linux cmd |
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
6527d76 to
f935bb4
Compare
|
All green. |
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
f935bb4 to
c7ae78e
Compare
topper-123
changed the title
topper-123
force-pushed
the
remove_rolling_etc_freq
branch
from
76864ba to
c7ae78e
Compare
topper-123
changed the title
|
thanks @topper-123 |
-- [x ] tests added / passed
git diff upstream/master -u -- "*.py" | flake8 --diffThe
freqparameter of df.rolling/expanding/ewm was deprecated in 0.18 (#11603). This PR removes the parameter from the code base.After this PR, I will remove the
howparameter and lastly thepd.rolling_*,pd.expanding_*andpd.ewm_*will be removed (AKApd.stats.*). By removingfreqandhowbeforepd.statsI think it will be easier to clean uppandas/tests/test_window.py, as ATM these three issues are not very cleanly separated in that test module.In some test in
test_window.py::TestMomentsthere is a bit of resampling going on, as I've movedfreqstuff fromrollinginto a priordf.resamplestep. These are tests forhowand will be removed oncehowis removed (unless the tests good for testing the windows functions, I'm not completely sure ATM, but will look into it when I reach that point).Additionally (and unrelated), in
pandas/tests/test_window.pythere are checks for numpy>=1.8 and >=1.9. These checks are no longer necessary, as numpy 1.9 is the current minium version, so they're removed,.