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

DEPR: Deprecate Series.valid #18800

Merged
merged 1 commit into from Dec 16, 2017

Conversation

Projects
None yet
4 participants
@topper-123
Contributor

topper-123 commented Dec 15, 2017

  • xref #18262
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

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 label Dec 15, 2017

@@ -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`).

This comment has been minimized.

@jreback

jreback Dec 15, 2017

Contributor

can you do :func:`Series.dropna`

This comment has been minimized.

@topper-123

topper-123 Dec 15, 2017

Contributor

changed to :meth:`Series.dropna.

@@ -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):

This comment has been minimized.

@jreback

jreback Dec 15, 2017

Contributor

add the issue number here

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

This comment has been minimized.

@topper-123

topper-123 Dec 15, 2017

Contributor

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

@jsexauer jsexauer referenced this pull request Dec 15, 2017

Open

DEPR: deprecations from prior versions #6581

0 of 81 tasks complete

@jreback jreback referenced this pull request Dec 15, 2017

Open

DEPR: let's deprecate #18262

12 of 36 tasks complete
@codecov

This comment has been minimized.

codecov bot commented Dec 15, 2017

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

This comment has been minimized.

pep8speaks commented Dec 15, 2017

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
tp
@jreback

This comment has been minimized.

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

This comment has been minimized.

Contributor

topper-123 commented Dec 16, 2017

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

This comment has been minimized.

Contributor

jreback commented Dec 16, 2017

no we always use FutureWarning
DeprecationWarning is not shown by default

@topper-123

This comment has been minimized.

Contributor

topper-123 commented Dec 16, 2017

All green

@jorisvandenbossche jorisvandenbossche merged commit f5415d8 into pandas-dev:master Dec 16, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@topper-123 topper-123 deleted the topper-123:deprecate_Series.valid branch Feb 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment