DEPR/CLN: Remove freq parameters from df.rolling/expanding/ewm #18601

topper-123 · 2017-12-02T18:19:55Z

-- [x ] tests added / passed

[x ] passes git diff upstream/master -u -- "*.py" | flake8 --diff
[x ] whatsnew entry

The freq parameter 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 how parameter and lastly the pd.rolling_*, pd.expanding_* and pd.ewm_* will be removed (AKA pd.stats.*). By removing freq and howbefore pd.stats I think it will be easier to clean up pandas/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::TestMoments there is a bit of resampling going on, as I've moved freq stuff from rolling into a prior df.resample step. These are tests for how and will be removed once how is 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.py there 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,.

gfyoung · 2017-12-03T08:41:30Z

doc/source/whatsnew/v0.22.0.txt

 - ``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)


Reference the original PR where it was deprecated.

gfyoung · 2017-12-03T08:42:29Z

@topper-123 : Thanks for this! Just need to patch a flake8 issue, and you should be good.

codecov · 2017-12-03T09:36:26Z

Codecov Report

Merging #18601 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            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

Flag	Coverage Δ
#multiple	`89.31% <100%> (-0.01%)`	⬇️
#single	`40.61% <77.77%> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/window.py	`96.31% <100%> (-0.07%)`	⬇️
pandas/stats/moments.py	`70.81% <100%> (-0.39%)`	⬇️
pandas/core/generic.py	`95.9% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.81% <0%> (-0.1%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e16818...010cf95. Read the comment docs.

codecov · 2017-12-03T09:36:38Z

Codecov Report

Merging #18601 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            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

Flag	Coverage Δ
#multiple	`89.44% <100%> (+0.01%)`	⬆️
#single	`40.68% <45%> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/window.py	`96.31% <100%> (-0.07%)`	⬇️
pandas/core/generic.py	`95.9% <100%> (ø)`	⬆️
pandas/stats/moments.py	`70.81% <100%> (-0.39%)`	⬇️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.81% <0%> (-0.1%)`	⬇️
pandas/plotting/_converter.py	`65.25% <0%> (+1.81%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3c04e2...c7ae78e. Read the comment docs.

topper-123 · 2017-12-03T11:34:51Z

all green. The travis error seems unrelated to this PR.

jreback

couple of comments

jreback · 2017-12-03T16:07:30Z

pandas/core/window.py

-    frequency by resampling the data. This is done with the default parameters
-    of :meth:`~pandas.Series.resample` (i.e. using the `mean`).
+    See Also
+    ---------


I think these need to be the same length as the text (the underlines)

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.

jreback · 2017-12-03T16:07:50Z

pandas/stats/moments.py

        if value is not None:
            kwds[k] = value

+    kwargs.pop('freq', None)  # freq removed in 0.22


this shouldn't be necessary

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.

that's fine, just add a TODO then

Well, the whole moments.py module will be deleted, incl. this small change....

But ok, I'ved added it.

I understand, and until that time, pls add a TODO

topper-123 · 2017-12-04T08:21:53Z

All green.

jreback · 2017-12-04T11:17:20Z

lint error & needs rebase

topper-123 · 2017-12-04T23:41:06Z

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 how, that is already mostly done.

jreback · 2017-12-04T23:43:52Z

linting is an issue on this PR
see the travis log

you can show it locally with
make lint-diff

topper-123 · 2017-12-05T00:17:35Z

Thanks, I've seen it on travis now.

Can't get make lint-diff to work (tried windows command prompt and MINGW64). is it a linux command?

jreback · 2017-12-05T00:33:49Z

yeah sorry it’s a linux cmd
the same command is ithe contributing guide and has a windows variant

topper-123 · 2017-12-05T06:47:22Z

All green.

jreback · 2017-12-06T01:13:57Z

thanks @topper-123

topper-123 force-pushed the remove_rolling_etc_freq branch from 0d5ae5d to 77b0b36 Compare December 2, 2017 18:21

gfyoung added Clean Deprecate Functionality to remove in pandas labels Dec 3, 2017

gfyoung reviewed Dec 3, 2017

View reviewed changes

jreback mentioned this pull request Dec 3, 2017

DEPR: deprecations log for removed issues #13777

Closed

topper-123 force-pushed the remove_rolling_etc_freq branch from 77b0b36 to 010cf95 Compare December 3, 2017 09:36

topper-123 force-pushed the remove_rolling_etc_freq branch 2 times, most recently from 03c04fe to b81fc11 Compare December 3, 2017 10:28

jreback requested changes Dec 3, 2017

View reviewed changes

topper-123 force-pushed the remove_rolling_etc_freq branch 2 times, most recently from 882d2c1 to 05c9006 Compare December 3, 2017 19:28

topper-123 force-pushed the remove_rolling_etc_freq branch from 05c9006 to 96899d7 Compare December 4, 2017 16:26

topper-123 force-pushed the remove_rolling_etc_freq branch from 96899d7 to 6527d76 Compare December 5, 2017 00:13

topper-123 force-pushed the remove_rolling_etc_freq branch from 6527d76 to f935bb4 Compare December 5, 2017 00:37

Remove freq keyword from df.rolling() etc.

c7ae78e

topper-123 force-pushed the remove_rolling_etc_freq branch from f935bb4 to c7ae78e Compare December 5, 2017 12:45

topper-123 changed the title ~~DEPR/CLN: Remove freq keyword from df.rolling/expanding/ewm~~ DEPR/CLN: Remove freq and how parameters from df.rolling/expanding/ewm Dec 5, 2017

topper-123 force-pushed the remove_rolling_etc_freq branch from 76864ba to c7ae78e Compare December 5, 2017 22:15

topper-123 changed the title ~~DEPR/CLN: Remove freq and how parameters from df.rolling/expanding/ewm~~ DEPR/CLN: Remove freq parameters from df.rolling/expanding/ewm Dec 5, 2017

jreback added this to the 0.22.0 milestone Dec 6, 2017

jreback approved these changes Dec 6, 2017

View reviewed changes

jreback merged commit 5665a3e into pandas-dev:master Dec 6, 2017

This was referenced Dec 6, 2017

DEPR/CLN: Remove how keyword from df.rolling() etc. #18668

Merged

DEPR/CLN: Remove pd.rolling_*, pd.expanding* and pd.ewm* #18723

Merged

topper-123 deleted the remove_rolling_etc_freq branch December 11, 2017 08:08

Uh oh!

DEPR/CLN: Remove freq parameters from df.rolling/expanding/ewm #18601

DEPR/CLN: Remove freq parameters from df.rolling/expanding/ewm #18601

Uh oh!

Conversation

topper-123 commented Dec 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gfyoung Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

gfyoung commented Dec 3, 2017

Uh oh!

codecov bot commented Dec 3, 2017

Codecov Report

Uh oh!

codecov bot commented Dec 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

topper-123 commented Dec 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jreback left a comment

Choose a reason for hiding this comment

Uh oh!

jreback Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

topper-123 Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

jreback Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

topper-123 Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

jreback Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

topper-123 Dec 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback Dec 3, 2017

Choose a reason for hiding this comment

Uh oh!

topper-123 commented Dec 4, 2017

Uh oh!

jreback commented Dec 4, 2017

Uh oh!

topper-123 commented Dec 4, 2017

Uh oh!

jreback commented Dec 4, 2017

Uh oh!

topper-123 commented Dec 5, 2017

Uh oh!

jreback commented Dec 5, 2017

Uh oh!

topper-123 commented Dec 5, 2017

Uh oh!

jreback commented Dec 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

topper-123 commented Dec 2, 2017 •

edited

Loading

codecov bot commented Dec 3, 2017 •

edited

Loading

topper-123 commented Dec 3, 2017 •

edited

Loading

topper-123 Dec 3, 2017 •

edited

Loading