REF: pandas/core/window.py into multiple files #27736

mroeschke · 2019-08-04T05:33:41Z

tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff

Splits pandas/core/window.py into more logical division of:

pandas/core/window/rolling.py
pandas/core/window/expanding.py
pandas/core/window/ewm.py
pandas/core/window/common.py

simonjayhawkins

@mroeschke lgtm. one question.

pandas/core/window/ewm.py

simonjayhawkins · 2019-08-04T08:29:11Z

pandas/core/window/expanding.py

+    _GroupByMixin,
+    _Rolling_and_Expanding,
+    _shared_docs,
+)


jbrockmendel · 2019-08-04T15:37:24Z

Doesn't need to be part of this PR, but could use your input: I've been wondering if it makes sense to put window and resample into groupby/, or otherwise organize in a way to denote that those two modules depend on groupby/

mroeschke · 2019-08-04T19:43:43Z

@jbrockmendel definitely. I imagine a lot of the aggregation functions could be collapsed as well if rolling, resample, and groupby lived under the same umbrella.

jreback · 2019-08-04T19:46:39Z

Doesn't need to be part of this PR, but could use your input: I've been wondering if it makes sense to put window and resample into groupby/, or otherwise organize in a way to denote that those two modules depend on groupby/

this would be too much currently: each of these is already a tremendous amount of tests and code
nesting more doesn’t help

jreback

lgtm, very light comments, ping on green.

jreback · 2019-08-04T21:18:35Z

pandas/core/generic.py

@@ -10690,9 +10690,13 @@ def _add_series_or_dataframe_operations(cls):
        the doc strings again.
        """

-        from pandas.core import window as rwindow
+        from pandas.core.window import (
+            ewm as lib_ewm,


we usually call the _libs imports like libwindow (etc), so is there a reason you can't just
from pandas.core.window import ewm, expanding, rolling? (or else I guess would be ok to call libewm, etc)

or just directly import what you need from these modules?

This caused a RecursionError since the defined functions here are:

def rolling(...): return rolling(...) cls.rolling = rolling

Okay I was able to use more direct imports, and all green.

simonjayhawkins · 2019-08-05T10:14:32Z

pandas/core/generic.py

@@ -10703,8 +10705,24 @@ def rolling(
            axis=0,
            closed=None,
        ):
+            if not isinstance(self, (ABCSeries, ABCDataFrame)):


why do we need this check? would prefer not to see ABCSeries and ABCDataFrame in core.generic.

yeah this should be inside Rolling constructor. Let's try to keep this diff as copy-paste as possible though & I don't think this was on the original diff at all?

This was in the original "top level" rolling function that would be imported here, but I found a better place to put this check.

jreback

lgtm other than some comments, ping on green.

jreback · 2019-08-05T11:37:29Z

pandas/core/generic.py

@@ -10703,8 +10705,24 @@ def rolling(
            axis=0,
            closed=None,
        ):
+            if not isinstance(self, (ABCSeries, ABCDataFrame)):


yeah this should be inside Rolling constructor. Let's try to keep this diff as copy-paste as possible though & I don't think this was on the original diff at all?

jreback · 2019-08-05T11:39:01Z

pandas/core/window/__init__.py

@@ -0,0 +1,3 @@
+from pandas.core.window.ewm import EWM  # noqa:F401
+from pandas.core.window.expanding import Expanding  # noqa:F401


I would add ExpandingGroupby & RollingGroupby here as well; we want to export the 'public' interface to the rest of pandas, so IOW, we don't have any where that needs to reach into pandas.core.window.*, then just import from pandas.core.window (this is all for inside of pandas)

jreback · 2019-08-05T11:40:01Z

pandas/tests/window/test_expanding.py

@@ -5,7 +5,7 @@

 import pandas as pd
 from pandas import DataFrame, Series
-import pandas.core.window as rwindow
+from pandas.core.window.expanding import Expanding


e.g. this should be
from pandas.core.window import Expanding

jreback · 2019-08-05T11:40:17Z

pandas/tests/window/test_ewm.py

@@ -4,7 +4,7 @@
 from pandas.errors import UnsupportedFunctionCall

 from pandas import DataFrame, Series
-import pandas.core.window as rwindow
+from pandas.core.window.ewm import EWM


from pandas.core.window import EWM

WillAyd · 2019-08-06T15:41:55Z

Note that #27767 was just merged so need to account for that in rebase

jreback · 2019-08-06T15:43:42Z

yep this needs a rebase, ping on green.

mroeschke · 2019-08-06T18:10:22Z

Ping all green.

jreback · 2019-08-07T13:33:07Z

thanks @mroeschke

did we have an issue for this? if so pls close it

REF: pandas/core/window.py

c6394e1

mroeschke added Refactor Internal refactoring of code Window rolling, ewma, expanding labels Aug 4, 2019

simonjayhawkins reviewed Aug 4, 2019

View reviewed changes

add more to window/common.py

68f7ca8

jreback requested changes Aug 4, 2019

View reviewed changes

jreback added this to the 1.0 milestone Aug 4, 2019

Matt Roeschke added 2 commits August 4, 2019 15:59

Add init imports for dask

bfdf37b

Use more direct imports

184c18d

simonjayhawkins reviewed Aug 5, 2019

View reviewed changes

jreback requested changes Aug 5, 2019

View reviewed changes

kernc mentioned this pull request Aug 6, 2019

BUG: Fix windowing over read-only arrays #27767

Merged

5 tasks

Matt Roeschke added 3 commits August 5, 2019 23:23

Merge remote-tracking branch 'upstream/master' into refactor_window

26724af

Add imports to window/__init__.py, put instance checks in validate

aff27ac

Simply more imports

69b5121

Merge remote-tracking branch 'upstream/master' into refactor_window

abd015b

jreback approved these changes Aug 7, 2019

View reviewed changes

jreback merged commit 3bf35c6 into pandas-dev:master Aug 7, 2019

mroeschke deleted the refactor_window branch August 7, 2019 16:53

This was referenced Aug 7, 2019

TRACKER: Related pandas-dev/pandas master PRs for Generalized Window Operations twosigma/pandas#4

Closed

Split pandas/core/window.py into pandas/core/window/*.py twosigma/pandas#7

Closed

quintusdias pushed a commit to quintusdias/pandas_dev that referenced this pull request Aug 16, 2019

REF: pandas/core/window.py into multiple files (pandas-dev#27736)

e0f445f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REF: pandas/core/window.py into multiple files #27736

REF: pandas/core/window.py into multiple files #27736

mroeschke commented Aug 4, 2019 •

edited

Loading

simonjayhawkins left a comment

simonjayhawkins Aug 4, 2019

jbrockmendel commented Aug 4, 2019

mroeschke commented Aug 4, 2019

jreback commented Aug 4, 2019

jreback left a comment

jreback Aug 4, 2019

jreback Aug 4, 2019

mroeschke Aug 4, 2019

mroeschke Aug 5, 2019

simonjayhawkins Aug 5, 2019

jreback Aug 5, 2019

mroeschke Aug 6, 2019

jreback left a comment

jreback Aug 5, 2019

jreback Aug 5, 2019

jreback Aug 5, 2019

jreback Aug 5, 2019

WillAyd commented Aug 6, 2019

jreback commented Aug 6, 2019

mroeschke commented Aug 6, 2019

jreback commented Aug 7, 2019

		@@ -0,0 +1,3 @@
		from pandas.core.window.ewm import EWM # noqa:F401
		from pandas.core.window.expanding import Expanding # noqa:F401

REF: pandas/core/window.py into multiple files #27736

REF: pandas/core/window.py into multiple files #27736

Conversation

mroeschke commented Aug 4, 2019 • edited Loading

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Aug 4, 2019

mroeschke commented Aug 4, 2019

jreback commented Aug 4, 2019

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd commented Aug 6, 2019

jreback commented Aug 6, 2019

mroeschke commented Aug 6, 2019

jreback commented Aug 7, 2019

mroeschke commented Aug 4, 2019 •

edited

Loading