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

Avoid setting warning filters globally #2744

Closed
astrojuanlu opened this issue Jun 28, 2023 · 0 comments · Fixed by #2747
Closed

Avoid setting warning filters globally #2744

astrojuanlu opened this issue Jun 28, 2023 · 0 comments · Fixed by #2747
Labels
Issue: Feature Request New feature or improvement to existing feature

Comments

@astrojuanlu
Copy link
Member

Description

We should avoid unconditionally setting warning filters globally, like we do here:

warnings.simplefilter("default", DeprecationWarning)

Context

The dozens of pkg_resources warnings have been bugging me forever. They can be seen everywhere: our Read the Docs builds https://readthedocs.org/projects/kedro/builds/21147135/, our tests, any kedro run that makes use of certain datasets, or just importing them:

❯ python -c "from kedro_datasets.pandas import CSVDataSet"                                                                                                                                                                                         (kedro38-dev) 
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:121: DeprecationWarning: pkg_resources is deprecated as an API
  warnings.warn("pkg_resources is deprecated as an API", DeprecationWarning)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('google')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('google.cloud')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2349: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('google')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(parent)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('google.logging')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('mpl_toolkits')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('ruamel')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('snowflake')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/pkg_resources/__init__.py:2870: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('sphinxcontrib')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  declare_namespace(pkg)
/Users/juan_cano/.micromamba/envs/kedro38-dev/lib/python3.8/site-packages/google/rpc/__init__.py:20: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('google.rpc')`.
Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
  pkg_resources.declare_namespace(__name__)

The problem is that, even if one wants to disable them, it doesn't work: nor from the CLI (with -Wignore) nor from the code itself. The reason is two-fold:

  1. The global registry overrides any other CLI option (as I painfully learned the hard way)
  2. The user might set up their own override, but if ours comes after, then it stands.

As a result, at the moment it's impossible to disable warnings coming from pkg_resources, which are hardly actionable for users.

Now, I probably get why this was done: because DeprecationWarnings do not get shown otherwise! That was the painful realization that led to PEP 565:

In Python 2.7 and Python 3.2, the default warning filters were updated to hide DeprecationWarning by default [...]
However, this change has had the unfortunate side effect of making DeprecationWarning markedly less effective at its primary intended purpose: providing advance notice of breaking changes in APIs (whether in CPython, the standard library, or in third party libraries) to users of those APIs.

At the moment (Python 3.7+), the default warning filters are:

default::DeprecationWarning:__main__
ignore::DeprecationWarning
ignore::PendingDeprecationWarning
ignore::ImportWarning
ignore::ResourceWarning

Which is probably why we decided to add a warnings.simplefilter("default", DeprecationWarning) to our code. However, this has too many side effects, and we should consider other ways.

Possible Implementation

I think we must remove all unconditional uses of warnings.simplefilter for standard Python warnings. Then, to still make our deprecation warnings visible, we could explore two possibilities:

  • Using FutureWarning instead of DeprecationWarning. The advantage is that FutureWarning is shown everywhere by default, so it would be more visible. In the old times, FutureWarning was supposed to be "for warnings about constructs that will change semantically in the future." (source), but from Python 3.7 onwards (post PEP 565) that's not the case anymore.
  • Creating a KedroDeprecationWarning class, inheriting from DeprecationWarning, and then have a single warnings.simplefilter("default", KedroDeprecationWarning) in our kedro/__init__.py.
  • Or, if we are to create a custom class, maybe we can just make it a child of FutureWarning. Would be weird to use Deprecation in a Future child, but maybe incurring in that internal inconsistency will minimize confusion while still making the intent clear.

Possible Alternatives

Alternatively, we could wrap our filter as advised by the CPython docs:

import sys

if not sys.warnoptions:
    import warnings
    warnings.simplefilter(...)

However, this would probably not solve the core issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue: Feature Request New feature or improvement to existing feature
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant