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.

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

Deprecate center on df.expanding #34887

Merged
merged 4 commits into from Jul 14, 2020
Merged

Deprecate center on df.expanding #34887

merged 4 commits into from Jul 14, 2020

Conversation

MBrouns
Copy link
Contributor

@MBrouns MBrouns commented Jun 20, 2020

df.expanding(center=True) currently returns non-sensical results and it is unclear what results would be expected. It was previously removed in #7934 for that same reason,but was reintroduced for unclear reasons in a later refactoring

@charlesdong1991 charlesdong1991 added Deprecate Functionality to remove in pandas Window rolling, ewma, expanding Bug labels Jun 20, 2020
Copy link
Member

@charlesdong1991 charlesdong1991 left a comment

Choose a reason for hiding this comment

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

thanks for active PRs! not very familiar with this deprecation scope, have a question to clear it.

pandas/core/window/expanding.py Outdated Show resolved Hide resolved
doc/source/whatsnew/v1.1.0.rst Show resolved Hide resolved
pandas/core/window/expanding.py Outdated Show resolved Hide resolved
@jreback
Copy link
Contributor

jreback commented Jun 20, 2020

you will either have to explicity set center=None for expanding tests, or use a @pytest.mark.filterwarnings("...") for this specific warning (this is ok to do)

pandas/core/window/expanding.py Outdated Show resolved Hide resolved
pandas/core/window/expanding.py Outdated Show resolved Hide resolved
@pep8speaks
Copy link

pep8speaks commented Jun 20, 2020

Hello @MBrouns! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-07-07 14:22:09 UTC

@MBrouns
Copy link
Contributor Author

MBrouns commented Jun 20, 2020

@jreback If I understood correctly you mean the change I made in f47dede

@jreback
Copy link
Contributor

jreback commented Jun 20, 2020

@jreback If I understood correctly you mean the change I made in f47dede

yep

@MBrouns
Copy link
Contributor Author

MBrouns commented Jun 26, 2020

@jreback anything that still needs to happen here?

@charlesdong1991
Copy link
Member

charlesdong1991 commented Jun 26, 2020

@MBrouns pls rebase and resolve conflicts, and make CI green. otherwise, it's unlikely that @jreback will review this PR

@MBrouns
Copy link
Contributor Author

MBrouns commented Jun 26, 2020

@charlesdong1991 the test failures seem to be unrelated to this PR and also seem to happen on master

@charlesdong1991
Copy link
Member

charlesdong1991 commented Jun 26, 2020

@MBrouns hey, after a quick skim, i think the linting failure is indeed on master, which will be fixed by #35007

but i think for this PR, still need to make changes for the test in pandas/tests/window/test_expanding.py or other test files in which the test uses expanding and raises the warning.

you need to either catch or ignore the warnings for the test, see https://github.com/pandas-dev/pandas/pull/34887/checks?check_run_id=810672133

`df.expanding(center=True)` currently returns non-sensical results and it
is unclear what results would be expected. It was previously removed in
#7934 for that same reason,
but was reintroduced for unclear reasons in a later refactoring
pandas/core/base.py Outdated Show resolved Hide resolved
pandas/core/base.py Outdated Show resolved Hide resolved
pandas/core/generic.py Show resolved Hide resolved
pandas/core/window/expanding.py Outdated Show resolved Hide resolved
pandas/tests/window/test_expanding.py Outdated Show resolved Hide resolved
@MBrouns MBrouns requested a review from jreback July 14, 2020 12:04
@jreback jreback added this to the 1.1 milestone Jul 14, 2020
@jreback jreback merged commit 1418ce8 into pandas-dev:master Jul 14, 2020
@jreback
Copy link
Contributor

jreback commented Jul 14, 2020

thanks @MBrouns very nice

fangchenli pushed a commit to fangchenli/pandas that referenced this pull request Jul 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Deprecate Functionality to remove in pandas Window rolling, ewma, expanding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API/BUG: center=True in expanding operations returns non-sensical results
5 participants