-
-
Notifications
You must be signed in to change notification settings - Fork 18k
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/REGR: Fix pandas.util.testing deprecation #30745
Conversation
Closes pandas-dev#30735 This avoids using _DeprecatedModule, which doesn't work for direct imports from a module.
pandas/util/__init__.py
Outdated
@@ -1,4 +1,60 @@ | |||
from pandas.util._decorators import Appender, Substitution, cache_readonly # noqa | |||
import importlib as _importlib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
umm why don't you actually just leave pandas.util.testing as a module itself? (and just re-import what you need). This is how we did all of the original deprecations.
I don't have a strong opinion on this approach vs just leaving the pandas.util.testing module and importing what is needed there (and add a deprecation warning in the module, which will only trigger a warning once?). But if we keep the approach in this PR, should this be factored out in a utility and just replace the current DeprecatedModule utility? |
Sorry, I think @jreback is right. We don't need the import hook stuff. Just # pandas/util/testing.py
warnings.warn("pandas.util.testing is deprecated. Use pandas.testing instead")
from pandas._testing import * should accomplish the same thing with less hassle. Will update shortly (I do think I need to restructure a few things to avoid a circular import from core) |
Sorry about that. Just using standard stuff now. |
LGTM |
with tm.assert_produces_warning(FutureWarning) as m: | ||
import pandas.util.testing as tm2 | ||
# avoid cache state affecting the test | ||
sys.modules.pop("pandas.util.testing", None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in theory this should be in a context manager (but nbd)
thanks @TomAugspurger |
Closes #30735
This avoids using _DeprecatedModule, which doesn't work for
direct imports from a module. Sorry for the importlib magic, but
I think this is the correct way to do things.
cc @jorisvandenbossche.