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

DEPR: pandas.util.testing #30620

Merged

Conversation

TomAugspurger
Copy link
Contributor

Closes #16232.

I tried to keep the commits somewhat clean.

e96624b and d52d35f can probably be ignored. That's just moving pandas.util.testing to pandas.util._testing and updating all the relevant imports.

Main question: do we want to make the tm.make* methods public? I'd say we probably should, but wanted to confirm.

@TomAugspurger TomAugspurger added this to the 1.0 milestone Jan 2, 2020
@TomAugspurger TomAugspurger added the Deprecate Functionality to remove in pandas label Jan 2, 2020
@TomAugspurger
Copy link
Contributor Author

On second thought, we may want to hold of on making the make* functions public. We'll want pep8 compatible names at least, possibly other changes.

@jbrockmendel
Copy link
Member

The naming is problematic for our isort config. We treat pandas.util._* as a collection of low-dependency modules, which tm is decidedly not. could go directly in pandas/_testing.py?

@TomAugspurger
Copy link
Contributor Author

Moved to pandas._testing.

@TomAugspurger
Copy link
Contributor Author

Not sure about the mypy failure.

mypy --version
mypy 0.730
Performing static analysis using mypy
pandas/tests/test_register_accessor.py:39: error: Module has no attribute "register_series_accessor"
pandas/tests/test_register_accessor.py:40: error: Module has no attribute "register_dataframe_accessor"
pandas/tests/test_register_accessor.py:41: error: Module has no attribute "register_index_accessor"
Found 3 errors in 1 file (checked 839 source files)

I saw this in #29964 too, but have never reproduced locally.

@jbrockmendel
Copy link
Member

jbrockmendel commented Jan 2, 2020 via email

@jreback
Copy link
Contributor

jreback commented Jan 3, 2020

lgtm.

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.

+1 merge on green.

@WillAyd
Copy link
Member

WillAyd commented Jan 3, 2020

Haven't seen the error before - does merging master resolve?

Main question: do we want to make the tm.make* methods public? I'd say we probably should, but wanted to confirm.

I don't think we want this. I'm under the impression these are better converted to fixtures

@TomAugspurger
Copy link
Contributor Author

Sounds good. Planning to merge this once it passes since it touch so many files.

@TomAugspurger
Copy link
Contributor Author

Another mypy failure.

@simonjayhawkins
Copy link
Member

Not sure about the mypy failure.

mypy --version
mypy 0.730
Performing static analysis using mypy
pandas/tests/test_register_accessor.py:39: error: Module has no attribute "register_series_accessor"
pandas/tests/test_register_accessor.py:40: error: Module has no attribute "register_dataframe_accessor"
pandas/tests/test_register_accessor.py:41: error: Module has no attribute "register_index_accessor"
Found 3 errors in 1 file (checked 839 source files)

I saw this in #29964 too, but have never reproduced locally.

i see this locally quite often on my check_untyped_defs branch. can normally clear with --no-incremental but that's not relevant for ci since there isn't a cache.

@TomAugspurger
Copy link
Contributor Author

Not sure if it's relevant, but we seem to only import pandas.api via pandas.io.json._table_schema, which doesn't seem great. I'm going to restructure things a bit in another PR.

@TomAugspurger
Copy link
Contributor Author

Passing now. Merging.

@TomAugspurger TomAugspurger merged commit c55a739 into pandas-dev:master Jan 3, 2020
@TomAugspurger TomAugspurger deleted the 16232-depr-util-testing branch January 3, 2020 19:34
@@ -26,6 +28,16 @@ Testing functions
testing.assert_frame_equal
testing.assert_series_equal
testing.assert_index_equal
testing.assert_equal
testing.assert_almost_equal
testing.assert_categorical_equal
Copy link
Member

Choose a reason for hiding this comment

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

Is it needed to make all those asserts public?
Eg, what's the pandas-specific value for external projects of using pandas' assert_equal or assert_almost_equal ?
Also all the EA specific ones could probably be limited to a single general array assert?

Copy link
Member

Choose a reason for hiding this comment

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

Thoughts about this? (can also make an issue for it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DEPR: pandas.core, pandas.util.testing
6 participants