BUG: Incorrect handling of rolling.cov with offset window #16244

keitakurita · 2017-05-04T22:33:50Z

closes Rolling.cov with offset window is expanding not rolling #16058
tests added / passed
passes git diff upstream/master --name-only -- '*.py' | flake8 --diff
whatsnew entry

jreback · 2017-05-04T23:28:08Z

pandas/tests/test_window.py

@@ -3396,6 +3396,18 @@ def test_min_periods(self):
        result = df.rolling('2s', min_periods=1).sum()
        tm.assert_frame_equal(result, expected)

+    def test_cov(self):
+


comment with the issue

codecov · 2017-05-05T00:36:21Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #16244      +/-   ##
==========================================
+ Coverage   90.34%   90.35%   +<.01%     
==========================================
  Files         164      164              
  Lines       50890    50894       +4     
==========================================
+ Hits        45977    45984       +7     
+ Misses       4913     4910       -3

Flag	Coverage Δ
#multiple	`88.13% <100%> (+0.02%)`	⬆️
#single	`40.3% <0%> (-0.11%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/window.py	`96.26% <100%> (+0.01%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.58% <0%> (-0.1%)`	⬇️
pandas/core/dtypes/common.py	`94.75% <0%> (ø)`	⬆️
pandas/plotting/_converter.py	`65.05% <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 2aa6e79...001a88b. Read the comment docs.

codecov · 2017-05-05T00:36:31Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #16244      +/-   ##
==========================================
+ Coverage   90.34%   90.35%   +<.01%     
==========================================
  Files         164      164              
  Lines       50890    50894       +4     
==========================================
+ Hits        45977    45984       +7     
+ Misses       4913     4910       -3

Flag	Coverage Δ
#multiple	`88.13% <100%> (+0.02%)`	⬆️
#single	`40.3% <0%> (-0.11%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/window.py	`96.26% <100%> (+0.01%)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.58% <0%> (-0.1%)`	⬇️
pandas/core/dtypes/common.py	`94.75% <0%> (ø)`	⬆️
pandas/plotting/_converter.py	`65.05% <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 2aa6e79...001a88b. Read the comment docs.

codecov · 2017-05-05T00:36:41Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #16244      +/-   ##
==========================================
+ Coverage   90.79%    90.8%   +<.01%     
==========================================
  Files         161      161              
  Lines       51063    51067       +4     
==========================================
+ Hits        46365    46369       +4     
  Misses       4698     4698

Flag	Coverage Δ
#multiple	`88.64% <100%> (ø)`	⬆️
#single	`40.15% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/window.py	`96.49% <100%> (+0.01%)`	⬆️

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 e60dc4c...a5582ec. Read the comment docs.

jreback

add the test from the issue. I would prefer that instead of comparing the results, you construct the correct results for each case.

add a whatsnew note in 0.20.2 bug fixes / rolling section

jreback · 2017-05-06T23:17:16Z

pandas/core/window.py

@@ -996,7 +997,10 @@ def cov(self, other=None, pairwise=None, ddof=1, **kwargs):
            # only default unset
            pairwise = True if pairwise is None else pairwise
        other = self._shallow_copy(other)
-        window = self._get_window(other)
+        if self.is_freq_type:


you can do this in _get_window itself i think.

I tried changing _get_window, but a lot of methods (such as var, median, and sum) require an integer window and seem to be working fine right now. If this issue is not a problem in methods other than cov, changing the code only for cov seems to make more sense.

test for issue and whatsnew note added

can you add a comment here (1-line) about this

jreback · 2017-05-19T01:08:44Z

pandas/tests/test_window.py

+        idx = pd.date_range('2017-01-01', periods=24, freq='1h')
+        ss = pd.Series(np.arange(len(idx)), index=idx)
+
+        result = ss.rolling('2h').cov()


also test again using ss.rolling(2).cov() (will be the same for this case).

I don't know if we have any tests for a non-regular date range, where freq=2 and freq='2h' are actually different.

TomAugspurger · 2017-05-30T21:41:14Z

Rebased. @jreback I think everything was addressed, if you want to verify?

jreback · 2017-05-30T23:12:54Z

thanks!

…#16244) (cherry picked from commit 4ca29f4)

(cherry picked from commit 4ca29f4)

…#16244)

Version 0.20.2 * tag 'v0.20.2': (68 commits) RLS: v0.20.2 DOC: Update release.rst DOC: Whatsnew fixups (pandas-dev#16596) ERRR: Raise error in usecols when column doesn't exist but length matches (pandas-dev#16460) BUG: convert numpy strings in index names in HDF pandas-dev#13492 (pandas-dev#16444) PERF: vectorize _interp_limit (pandas-dev#16592) DOC: whatsnew 0.20.2 edits (pandas-dev#16587) API: Make is_strictly_monotonic_* private (pandas-dev#16576) BUG: reimplement MultiIndex.remove_unused_levels (pandas-dev#16565) Strictly monotonic (pandas-dev#16555) ENH: add .ngroup() method to groupby objects (pandas-dev#14026) (pandas-dev#14026) fix linting BUG: Incorrect handling of rolling.cov with offset window (pandas-dev#16244) BUG: select_as_multiple doesn't respect start/stop kwargs GH16209 (pandas-dev#16317) return empty MultiIndex for symmetrical difference on equal MultiIndexes (pandas-dev#16486) BUG: Bug in .resample() and .groupby() when aggregating on integers (pandas-dev#16549) BUG: Fixed tput output on windows (pandas-dev#16496) Strictly monotonic (pandas-dev#16555) BUG: fixed wrong order of ordered labels in pd.cut() BUG: Fixed to_html ignoring index_names parameter ...

jreback reviewed May 4, 2017

View reviewed changes

jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode Datetime Datetime data dtype labels May 4, 2017

keitakurita force-pushed the rolling_window_cov branch from d782f55 to 001a88b Compare May 5, 2017 00:35

jreback requested changes May 6, 2017

View reviewed changes

keitakurita force-pushed the rolling_window_cov branch 3 times, most recently from 0e6ccaf to 76024c1 Compare May 13, 2017 13:12

jreback added this to the 0.20.2 milestone May 19, 2017

jreback reviewed May 19, 2017

View reviewed changes

keitakurita force-pushed the rolling_window_cov branch from 76024c1 to 1c34441 Compare May 20, 2017 00:25

BUG: Incorrect handling of rolling.cov with offset window

a5582ec

TomAugspurger force-pushed the rolling_window_cov branch from 1c34441 to a5582ec Compare May 30, 2017 21:39

jreback approved these changes May 30, 2017

View reviewed changes

jreback merged commit 4ca29f4 into pandas-dev:master May 30, 2017

TomAugspurger added the Needs Backport label Jun 1, 2017

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Jun 1, 2017

BUG: Incorrect handling of rolling.cov with offset window (pandas-dev…

19330c2

…#16244) (cherry picked from commit 4ca29f4)

TomAugspurger pushed a commit that referenced this pull request Jun 4, 2017

BUG: Incorrect handling of rolling.cov with offset window (#16244)

a14e338

(cherry picked from commit 4ca29f4)

TomAugspurger removed the Needs Backport label Jun 4, 2017

Kiv pushed a commit to Kiv/pandas that referenced this pull request Jun 11, 2017

BUG: Incorrect handling of rolling.cov with offset window (pandas-dev…

1a9cb5b

…#16244)

stangirala pushed a commit to stangirala/pandas that referenced this pull request Jun 11, 2017

BUG: Incorrect handling of rolling.cov with offset window (pandas-dev…

42e370d

…#16244)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Incorrect handling of rolling.cov with offset window #16244

BUG: Incorrect handling of rolling.cov with offset window #16244

keitakurita commented May 4, 2017 •

edited

Loading

jreback May 4, 2017

keitakurita May 5, 2017

codecov bot commented May 5, 2017 •

edited

Loading

codecov bot commented May 5, 2017

codecov bot commented May 5, 2017 •

edited

Loading

jreback left a comment

jreback May 6, 2017

keitakurita May 8, 2017

keitakurita May 8, 2017

jreback May 19, 2017

jreback May 19, 2017 •

edited

Loading

keitakurita May 20, 2017

TomAugspurger commented May 30, 2017

jreback commented May 30, 2017

BUG: Incorrect handling of rolling.cov with offset window #16244

BUG: Incorrect handling of rolling.cov with offset window #16244

Conversation

keitakurita commented May 4, 2017 • edited Loading

jreback May 4, 2017

Choose a reason for hiding this comment

keitakurita May 5, 2017

Choose a reason for hiding this comment

codecov bot commented May 5, 2017 • edited Loading

Codecov Report

codecov bot commented May 5, 2017

Codecov Report

codecov bot commented May 5, 2017 • edited Loading

Codecov Report

jreback left a comment

Choose a reason for hiding this comment

jreback May 6, 2017

Choose a reason for hiding this comment

keitakurita May 8, 2017

Choose a reason for hiding this comment

keitakurita May 8, 2017

Choose a reason for hiding this comment

jreback May 19, 2017

Choose a reason for hiding this comment

jreback May 19, 2017 • edited Loading

Choose a reason for hiding this comment

keitakurita May 20, 2017

Choose a reason for hiding this comment

TomAugspurger commented May 30, 2017

jreback commented May 30, 2017

keitakurita commented May 4, 2017 •

edited

Loading

codecov bot commented May 5, 2017 •

edited

Loading

codecov bot commented May 5, 2017 •

edited

Loading

jreback May 19, 2017 •

edited

Loading