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

rolling.quantile returns an interpolated result #16216

Closed
wants to merge 15 commits into from
Closed

rolling.quantile returns an interpolated result #16216

wants to merge 15 commits into from

Conversation

guillemborrell
Copy link
Contributor

@guillemborrell guillemborrell commented May 3, 2017 •
edited
Loading

@guillemborrell
Rolling window quantile defaults fixed
89abf89 
Now the rolling window quantile returns the interpolated value
like np.percentile and Series.quantile.
@guillemborrell
Fixed rolling window tests too
4ae3d88
@guillemborrell
Corrected small glitch in test
4927e2f
@guillemborrell guillemborrell mentioned this pull request May 3, 2017
Closed
jreback
jreback requested changes May 3, 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.

pls add a release note (0.20.1), call a bug fix is fine.
pls add the example from the issue as an additional tests; add a comment to reference the issue

pandas/tests/test_window.py Outdated
qhig = (int(idx) + 1) / (values.shape[0] - 1)
vlow = values[int(idx)]
vhig = values[int(idx + 1)]
retval = vlow + (vhig - vlow)*(per - qlow)/(qhig - qlow)
Copy link
Contributor

Choose a reason for hiding this comment

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

this will cause some lint issues

@jreback
Copy link
Contributor

jreback commented May 3, 2017

add the examples as tests from #9413

@jreback jreback added Numeric Operations Arithmetic, Comparison, and Logical operations Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels May 3, 2017
@codecov
Copy link

codecov bot commented May 3, 2017

Codecov Report

Merging #16216 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #16216   +/-   ##
=======================================
  Coverage   90.89%   90.89%           
=======================================
  Files         162      162           
  Lines       50884    50884           
=======================================
  Hits        46250    46250           
  Misses       4634     4634
Flag Coverage Δ
#multiple 88.67% <ø> (ø) ⬆️
#single 40.31% <ø> (ø) ⬆️

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 f154966...ba5c751. Read the comment docs.

@codecov
Copy link

codecov bot commented May 3, 2017 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16216      +/-   ##
==========================================
- Coverage   90.25%   90.23%   -0.02%     
==========================================
  Files         164      162       -2     
  Lines       50894    50882      -12     
==========================================
- Hits        45934    45914      -20     
- Misses       4960     4968       +8
Flag Coverage Δ
#multiple 88.02% <ø> (-0.01%) ⬇️
#single 40.31% <ø> (-0.1%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/conftest.py 95.45% <0%> (-0.38%) ⬇️
pandas/core/frame.py 97.58% <0%> (-0.1%) ⬇️
pandas/util/testing.py 78.78% <0%> (-0.09%) ⬇️
pandas/io/clipboard/clipboards.py
pandas/io/clipboard/__init__.py
pandas/util/_decorators.py
pandas/util/__init__.py
pandas/util/_validators.py
pandas/io/clipboard/exceptions.py
... and 22 more

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 2002da3...1274fcc. Read the comment docs.

@guillemborrell
Copy link
Contributor Author

guillemborrell commented May 3, 2017 •
edited
Loading

@jreback I think that I have implemented all the requested improvements.

EDIT: Some tests failed in circleci, but ran OK in my machine.

@guillemborrell
Copy link
Contributor Author

@jreback. Tests passed and changes implemented.

@guillemborrell
Copy link
Contributor Author

@jreback I closed this a created a clean PR: #16247

@jreback
Copy link
Contributor

jreback commented May 5, 2017

pls don't open new pr's for the same issue, you simply push to the same PR. ok on this one.

@guillemborrell
Copy link
Contributor Author

Sorry. Just trying to make things right.

@TomAugspurger TomAugspurger added this to the No action milestone May 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback requested changes

Assignees
No one assigned
Labels
Numeric Operations Arithmetic, Comparison, and Logical operations Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BUG: pandas rolling_quantile does not use interpolation
3 participants
@guillemborrell @jreback @TomAugspurger