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: remove pandas.core.common is_* #19769

Merged
merged 3 commits into from Feb 22, 2018

Conversation

Projects
None yet
4 participants
@jreback
Contributor

jreback commented Feb 19, 2018

xref #13990
xref #19304

@jreback jreback added the Deprecate label Feb 19, 2018

@jreback jreback added this to the 0.23.0 milestone Feb 19, 2018

@@ -624,6 +624,7 @@ Removal of prior version deprecations/changes
- The modules `pandas.tools.hashing` and `pandas.util.hashing` have been removed (:issue:`16223`)
- The top-level functions ``pd.rolling_*``, ``pd.expanding_*`` and ``pd.ewm*`` have been removed (Deprecated since v0.18).
Instead, use the DataFrame/Series methods :attr:`~DataFrame.rolling`, :attr:`~DataFrame.expanding` and :attr:`~DataFrame.ewm` (:issue:`18723`)
- Imports from ``pandas.core.common`` for api like functions such as ``is_datetime64_dtype`` are now removed. These are located in ``pandas.api.types``. Furthermore, errors are removed as well from ``pandas.core.common`` and are available in ``pandas.errors`` (:issue:`13990`)

This comment has been minimized.

@TomAugspurger

TomAugspurger Feb 19, 2018

Contributor

Can remove "for api" and just have "functions like is_datetime64_dtype."

And using the errors here didn't actually emit deprecation warnings did it? Those should be deprecated before removing.

This comment has been minimized.

@jreback

jreback Feb 19, 2018

Contributor

no, we simply have to remove them. these are classes and we were never able to actually deprecate them.

This comment has been minimized.

@TomAugspurger

TomAugspurger Feb 19, 2018

Contributor

You can deprecate pretty easily with subclassing

class PerformanceWarning(pandas.errors.PerformanceWarning):
    def __init__(self, *args, **kwargs):
        warnings.warn('Deprecated. Use pandas.errors instead', FutureWarning)
        super().__init__(*args, **kwargs)

could meta-program that if you want, but it's just 4 errors.

This comment has been minimized.

@jreback

jreback Feb 19, 2018

Contributor

I believe @jorisvandenbossche tried this, and we had a discussion about this in the original issue.

This comment has been minimized.

@jorisvandenbossche

jorisvandenbossche Feb 19, 2018

Member

The problem we discussed AFAIR was about the problem people importing an error to then only catch it. That way the error is never 'called', and a deprecation warning like the above would not work.
It does work for raising such a warning (but I think catching will be more common).

This comment has been minimized.

@jorisvandenbossche

jorisvandenbossche Feb 19, 2018

Member

I think the only way this can really be solved is by renaming core/common.py to eg core/utils.py, and then you can deprecate the whole core/common.py module, and already raise warning on importing it.

This comment has been minimized.

@jbrockmendel

jbrockmendel Feb 19, 2018

Member

Could override isinstancecheck? Probably overkill.

This comment has been minimized.

@jreback

jreback Feb 19, 2018

Contributor

yeah I think we had this discussion a while back, and it looks like it can be done, but then the details kill us. @jorisvandenbossche soln would work, but I think will just put a big warning.

@TomAugspurger

This comment has been minimized.

Contributor

TomAugspurger commented Feb 19, 2018

@codecov

This comment has been minimized.

codecov bot commented Feb 19, 2018

Codecov Report

Merging #19769 into master will decrease coverage by 0.04%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #19769      +/-   ##
==========================================
- Coverage   91.61%   91.57%   -0.05%     
==========================================
  Files         150      150              
  Lines       48887    48865      -22     
==========================================
- Hits        44786    44746      -40     
- Misses       4101     4119      +18
Flag Coverage Δ
#multiple 89.94% <50%> (-0.05%) ⬇️
#single 41.79% <16.66%> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/core/common.py 92.9% <ø> (-0.52%) ⬇️
pandas/core/base.py 96.8% <40%> (ø) ⬆️
pandas/core/resample.py 96.43% <57.14%> (ø) ⬆️
pandas/plotting/_converter.py 65.22% <0%> (-1.74%) ⬇️
pandas/core/dtypes/common.py 93.91% <0%> (-1.46%) ⬇️

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 e022d9a...56017ca. Read the comment docs.

@jreback jreback merged commit 842d350 into pandas-dev:master Feb 22, 2018

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

@jreback jreback referenced this pull request Feb 22, 2018

Open

DEPR: deprecations log for removed issues #13777

115 of 115 tasks complete

harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018

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