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

DEPR: Deprecate Series.valid #18800

Merged
merged 1 commit into from Dec 16, 2017
Merged

DEPR: Deprecate Series.valid #18800

merged 1 commit into from Dec 16, 2017

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented Dec 15, 2017 •
edited

This PR deprecates the valid method on pd.Series. This method duplicates dropna and should be deprecated according to #18262.

@jreback jreback added the Deprecate Functionality to remove in pandas label Dec 15, 2017
jreback
jreback requested changes Dec 15, 2017
View reviewed changes
doc/source/whatsnew/v0.22.0.txt Outdated
@@ -203,6 +203,7 @@ Deprecations
- ``Series.from_array`` and ``SparseSeries.from_array`` are deprecated. Use the normal constructor ``Series(..)`` and ``SparseSeries(..)`` instead (:issue:`18213`).
- ``DataFrame.as_matrix`` is deprecated. Use ``DataFrame.values`` instead (:issue:`18458`).
- ``Series.asobject``, ``DatetimeIndex.asobject``, ``PeriodIndex.asobject`` and ``TimeDeltaIndex.asobject`` have been deprecated. Use ``.astype(object)`` instead (:issue:`18572`)
- ``Series.valid`` is deprecated. Use ``Series.dropna`` instead (:issue:`18800`).
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 do :func:`Series.dropna`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to :meth:`Series.dropna.

pandas/tests/generic/test_generic.py Outdated
@@ -581,6 +581,10 @@ def test_validate_bool_args(self):
with pytest.raises(ValueError):
super(DataFrame, df).mask(cond=df.a > 2, inplace=value)

def test_valid_deprecated(self):
with tm.assert_produces_warning(FutureWarning):
Copy link
Contributor

Choose a reason for hiding this comment

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

add the issue number here

not sure this is actually in the right place. these are fairly generic routines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I've moved the test to tests/generic/test_series.py.

@jsexauer jsexauer mentioned this pull request Dec 15, 2017
Closed
1 task
@jreback jreback mentioned this pull request Dec 15, 2017
Closed
34 tasks
@codecov
Copy link

codecov bot commented Dec 15, 2017 •
edited

Codecov Report

Merging #18800 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18800      +/-   ##
==========================================
- Coverage   91.63%   91.62%   -0.02%     
==========================================
  Files         154      154              
  Lines       51422    51430       +8     
==========================================
- Hits        47123    47122       -1     
- Misses       4299     4308       +9
Flag Coverage Δ
#multiple 89.49% <100%> (ø) ⬆️
#single 40.83% <25%> (-0.1%) ⬇️
Impacted Files Coverage Δ
pandas/core/series.py 94.82% <100%> (ø) ⬆️
pandas/core/sparse/series.py 95.33% <100%> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.68% <0%> (-0.11%) ⬇️
pandas/core/generic.py 95.9% <0%> (ø) ⬆️
pandas/util/_test_decorators.py 94.54% <0%> (+0.66%) ⬆️

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 0303d0d...8c534ca. Read the comment docs.

@pep8speaks
Copy link

pep8speaks commented Dec 15, 2017 •
edited

Hello @topper-123! Thanks for updating the PR.

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

Comment last updated on December 15, 2017 at 23:40 Hours UTC

@jreback
Copy link
Contributor

jreback commented Dec 15, 2017

lgtm. ping on green.

@jreback jreback added this to the 0.22.0 milestone Dec 15, 2017
@topper-123
Copy link
Contributor Author

I'm using FutureWarning as the warning type. Should this not be DeprecationWarning instead? I could update my previous deprecation commits also, while I'm at it.

@jreback
Copy link
Contributor

jreback commented Dec 16, 2017

no we always use FutureWarning
DeprecationWarning is not shown by default

@topper-123
Copy link
Contributor Author

All green

@jorisvandenbossche jorisvandenbossche merged commit f5415d8 into pandas-dev:master Dec 16, 2017
@topper-123 topper-123 deleted the deprecate_Series.valid branch February 2, 2018 17:06
@jreback jreback mentioned this pull request Nov 21, 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

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Milestone
0.23.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@topper-123 @pep8speaks @jreback @jorisvandenbossche