Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUG: Fix inaccurate rolling.var calculation #18481

Merged
merged 7 commits into from
Nov 25, 2017
Merged

BUG: Fix inaccurate rolling.var calculation #18481

merged 7 commits into from
Nov 25, 2017

Conversation

Licht-T
Copy link
Contributor

@Licht-T Licht-T commented Nov 25, 2017 •
edited
Loading

jreback
jreback requested changes Nov 25, 2017
View reviewed changes
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 a whatsnew note 0.21.1 bug fix

pandas/tests/test_window.py Outdated
s = pd.Series(np.zeros(20))
other = pd.Series(np.arange(20))

assert s.rolling(window=5).corr(other=other).isna().all()
Copy link
Contributor

Choose a reason for hiding this comment

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

and for window 0 thru 6

jreback
jreback reviewed Nov 25, 2017
View reviewed changes
pandas/_libs/window.pyx Outdated
mean_x[0] = mean_x[0] - delta / nobs[0]
ssqdm_x[0] = ssqdm_x[0] - delta * (val - mean_x[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

add sone commentary that u r using welford online algos- add a link here

@jreback jreback added Bug Numeric Operations Arithmetic, Comparison, and Logical operations labels Nov 25, 2017
@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

@byospe can you have a look.

@codecov
Copy link

codecov bot commented Nov 25, 2017 •
edited
Loading

Codecov Report

Merging #18481 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18481      +/-   ##
==========================================
- Coverage   91.34%    91.3%   -0.05%     
==========================================
  Files         163      163              
  Lines       49717    49717              
==========================================
- Hits        45413    45392      -21     
- Misses       4304     4325      +21
Flag Coverage Δ
#multiple 89.09% <ø> (-0.03%) ⬇️
#single 40.51% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/plotting/_converter.py 63.44% <0%> (-1.82%) ⬇️
pandas/core/frame.py 97.8% <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 412988e...b5b664c. Read the comment docs.

@codecov
Copy link

codecov bot commented Nov 25, 2017 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@b69c1a2). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #18481   +/-   ##
=========================================
  Coverage          ?   91.32%           
=========================================
  Files             ?      163           
  Lines             ?    49752           
  Branches          ?        0           
=========================================
  Hits              ?    45434           
  Misses            ?     4318           
  Partials          ?        0
Flag Coverage Δ
#multiple 89.12% <ø> (?)
#single 40.71% <ø> (?)

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 b69c1a2...b31558d. Read the comment docs.

@Licht-T Licht-T force-pushed the fix-inaccurate-var-calc branch from b5b664c to b8e84e3 Compare November 25, 2017 15:12
@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 25, 2017

@jreback Fixed!

@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

thanks @Licht-T can you rebase once again.

@Licht-T
Copy link
Contributor Author

Licht-T commented Nov 25, 2017

@jreback Rebased.

@jreback jreback added this to the 0.21.1 milestone Nov 25, 2017
@jreback jreback merged commit 3d44221 into pandas-dev:master Nov 25, 2017
@jreback
Copy link
Contributor

jreback commented Nov 25, 2017

thanks @Licht-T keep em coming!

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017
@Licht-T @TomAugspurger
BUG: Fix inaccurate rolling.var calculation (pandas-dev#18481)
cbe24b6 
(cherry picked from commit 3d44221)
TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017
@Licht-T @TomAugspurger
BUG: Fix inaccurate rolling.var calculation (#18481)
604fcb6 
(cherry picked from commit 3d44221)
@mroeschke mroeschke mentioned this pull request Oct 21, 2018
Closed
@ghost ghost mentioned this pull request Jul 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug Numeric Operations Arithmetic, Comparison, and Logical operations
Projects
None yet
Milestone
0.21.1
Development

Successfully merging this pull request may close these issues.

BUG: rolling.corr() produces wrong result with equal values
3 participants
@Licht-T @jreback @TomAugspurger