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

Deprecate series.nonzero (GH18262) #24048

Merged
merged 20 commits into from
Jan 5, 2019
Merged

Deprecate series.nonzero (GH18262) #24048

merged 20 commits into from
Jan 5, 2019

Conversation

makbigc
Copy link
Contributor

@makbigc makbigc commented Dec 2, 2018

xref #18262

  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@pep8speaks
Copy link

pep8speaks commented Dec 2, 2018 •
edited
Loading

Hello @makbigc! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on January 05, 2019 at 21:23 Hours UTC

@jreback
Copy link
Contributor

jreback commented Dec 2, 2018

this would need a test. Also you would need to remove any uses of this from the test code itself (otherwise it would show a warning)

jreback
jreback requested changes Dec 2, 2018
View reviewed changes
pandas/core/series.py
@@ -564,6 +566,9 @@ def nonzero(self):
--------
numpy.nonzero
"""
msg = ("Series.nonzero() is deprecated "
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 show what it is replaced by here

@jreback jreback added the Deprecate Functionality to remove in pandas label Dec 2, 2018
@jreback
Copy link
Contributor

jreback commented Dec 14, 2018

can you merge master and update

@codecov
Copy link

codecov bot commented Dec 30, 2018

Codecov Report

Merging #24048 into master will decrease coverage by 49.84%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #24048       +/-   ##
===========================================
- Coverage   92.31%   42.46%   -49.85%     
===========================================
  Files         166      161        -5     
  Lines       52412    51559      -853     
===========================================
- Hits        48382    21894    -26488     
- Misses       4030    29665    +25635
Flag Coverage Δ
#multiple ?
#single 42.46% <100%> (-0.6%) ⬇️
Impacted Files Coverage Δ
pandas/core/series.py 50.83% <100%> (-42.91%) ⬇️
pandas/io/formats/latex.py 0% <0%> (-100%) ⬇️
pandas/core/categorical.py 0% <0%> (-100%) ⬇️
pandas/io/sas/sas_constants.py 0% <0%> (-100%) ⬇️
pandas/tseries/plotting.py 0% <0%> (-100%) ⬇️
pandas/tseries/converter.py 0% <0%> (-100%) ⬇️
pandas/io/formats/html.py 0% <0%> (-98.65%) ⬇️
pandas/core/groupby/categorical.py 0% <0%> (-95.46%) ⬇️
pandas/io/sas/sas7bdat.py 0% <0%> (-91.17%) ⬇️
pandas/io/sas/sas_xport.py 0% <0%> (-90.15%) ⬇️
... and 129 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 d5e5bf7...e06a908. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 30, 2018 •
edited
Loading

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #24048      +/-   ##
==========================================
+ Coverage   92.37%   92.37%   +<.01%     
==========================================
  Files         166      166              
  Lines       52377    52379       +2     
==========================================
+ Hits        48385    48387       +2     
  Misses       3992     3992
Flag Coverage Δ
#multiple 90.8% <100%> (ø) ⬆️
#single 43.02% <25%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/series.py 93.68% <100%> (+0.01%) ⬆️
pandas/core/frame.py 96.92% <100%> (ø) ⬆️

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 280a88f...541e816. Read the comment docs.

jreback
jreback requested changes Dec 30, 2018
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.

are there any warnings issued by the test suite? need to change any internal uses of this

pandas/core/frame.py Outdated Show resolved Hide resolved
pandas/core/frame.py Outdated Show resolved Hide resolved
@jsexauer jsexauer mentioned this pull request Dec 30, 2018
Closed
1 task
@jreback
Copy link
Contributor

jreback commented Dec 30, 2018

also remove this from api.rst if it exists

@jreback
Copy link
Contributor

jreback commented Jan 1, 2019

the original docs/api.rst got added back, can you remove

jreback
jreback requested changes Jan 1, 2019
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.

there are NO warnings from the test suite?

pandas/core/frame.py Outdated Show resolved Hide resolved
pandas/core/series.py Outdated
@@ -569,6 +571,10 @@ def nonzero(self):
d 4
dtype: int64
"""
msg = ("Series.nonzero() is deprecated "
"and will be removed in a future version."
"Use np.nonzero(Series.values) instead")
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not a good replacement, instead recommend
np.nonzero(Series.to_numpy())(or Series.to_numpy().non_zero())

pandas/io/stata.py Outdated Show resolved Hide resolved
pandas/tests/frame/test_indexing.py Outdated Show resolved Hide resolved
@makbigc
Copy link
Contributor Author

makbigc commented Jan 5, 2019

No warning is raised due to deprecation of Series.nonzero, running the test suite in my own machine.

jreback
jreback requested changes Jan 5, 2019
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.

looks fine, but need to remove doc/source/api.rst which you seem to have added back

pandas/core/series.py Show resolved Hide resolved
@jreback jreback added this to the 0.24.0 milestone Jan 5, 2019
@jreback jreback mentioned this pull request Jan 5, 2019
Closed
34 tasks
@jreback jreback merged commit dc91f4c into pandas-dev:master Jan 5, 2019
@jreback
Copy link
Contributor

jreback commented Jan 5, 2019

thank @makbigc

@rs2 rs2 mentioned this pull request Feb 2, 2019
Closed
samuelsinayoko added a commit to samuelsinayoko/pandas that referenced this pull request Feb 2, 2019
@samuelsinayoko
Revert earlier change and use to_numpy
2aca389 
Revert pandas-dev#24048 change that caused bug.
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@makbigc @Pingviinituutti
Deprecate series.nonzero (GH18262) (pandas-dev#24048)
eab04c0
Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019
@makbigc @Pingviinituutti
Deprecate series.nonzero (GH18262) (pandas-dev#24048)
513ff84
@jreback jreback mentioned this pull request Dec 1, 2019
Closed
@charignon charignon mentioned this pull request Feb 15, 2020
Closed
charignon added a commit to charignon/prophet that referenced this pull request Feb 15, 2020
@charignon
Remove deprecated usage of Series.nonzero()
0485991 
Series.nonzero() was deprecated in pandas-dev/pandas#24048.

Using the latest pandas breaks fbprophet.

We can replace .nonzero() use with .to_numpy().nonzero().
charignon added a commit to charignon/prophet that referenced this pull request Feb 16, 2020
@charignon
Remove deprecated usage of Series.nonzero()
b5421c8 
Series.nonzero() was deprecated in pandas-dev/pandas#24048.

Using the latest pandas breaks fbprophet.

We can replace .nonzero() use with .to_numpy().nonzero().
@charignon charignon mentioned this pull request Feb 16, 2020
Closed
1 task
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
Deprecate Functionality to remove in pandas
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.
3 participants
@makbigc @pep8speaks @jreback