Skip to content

Conversation

keitakurita
Copy link
Contributor

@keitakurita keitakurita commented May 4, 2017

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment with the issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

@jreback jreback added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode Datetime Datetime data dtype labels May 4, 2017
@keitakurita keitakurita force-pushed the rolling_window_cov branch from d782f55 to 001a88b Compare May 5, 2017 00:35
@codecov
Copy link

codecov bot commented May 5, 2017

Codecov Report

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

Impacted file tree graph

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

codecov bot commented May 5, 2017

Codecov Report

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

Impacted file tree graph

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

codecov bot commented May 5, 2017

Codecov Report

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

Impacted file tree graph

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

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can do this in _get_window itself i think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test for issue and whatsnew note added

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@keitakurita keitakurita force-pushed the rolling_window_cov branch 3 times, most recently from 0e6ccaf to 76024c1 Compare May 13, 2017 13:12
@jreback jreback added this to the 0.20.2 milestone May 19, 2017
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()
Copy link
Contributor

@jreback jreback May 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test added

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

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

@jreback jreback merged commit 4ca29f4 into pandas-dev:master May 30, 2017
@jreback
Copy link
Contributor

jreback commented May 30, 2017

thanks!

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Jun 1, 2017
TomAugspurger pushed a commit that referenced this pull request Jun 4, 2017
stangirala pushed a commit to stangirala/pandas that referenced this pull request Jun 11, 2017
yarikoptic added a commit to neurodebian/pandas that referenced this pull request Jul 12, 2017
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
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Datetime Datetime data dtype Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rolling.cov with offset window is expanding not rolling
4 participants