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

[Docs] 70.0.0 should state breaking change of removal of packaging.pkg_resources #4376

Closed
1 task done
davidxia opened this issue May 21, 2024 · 4 comments
Closed
1 task done
Labels
documentation Needs Triage Issues that need to be evaluated for severity and status.

Comments

@davidxia
Copy link

Summary

69.5.1 had packaging.pkg_resources. I can run from pkg_resources import packaging.

But 70.0.0 seems to remove packaging.pkg_resources. Running from pkg_resources import packaging errors with ImportError: cannot import name 'packaging' from 'pkg_resources'.

Should this breaking change be stated in the changelog?

OS / Environment

tested on Linux Ubuntu 20.04.6 LTS, Python 3.10.13

Additional Information

minimal repro

from pkg_resources import packaging works with 69.5.1

python -c 'import setuptools; print(setuptools.__version__); from pkg_resources import packaging'
69.5.1
<string>:1: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html

python -c 'import setuptools; print(setuptools.__version__); from pkg_resources import packaging; print("SUCCESS")'
69.5.1
<string>:1: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
SUCCESS

from pkg_resources import packaging fails with 70.0.0

python -m pip install --upgrade setuptools
Looking in indexes: https://pypi.spotify.net/simple/
Requirement already satisfied: setuptools in /home/dxia/.pyenv/versions/3.9.18/envs/prolific-fm/lib/python3.10/site-packages (69.5.1)
Collecting setuptools
  Using cached https://artifactory.spotify.net/artifactory/api/pypi/pypi/packages/packages/de/88/70c5767a0e43eb4451c2200f07d042a4bcd7639276003a9c54a68cfcc1f8/setuptools-70.0.0-py3-none-any.whl (863 kB)
Installing collected packages: setuptools
  Attempting uninstall: setuptools
    Found existing installation: setuptools 69.5.1
    Uninstalling setuptools-69.5.1:
      Successfully uninstalled setuptools-69.5.1
Successfully installed setuptools-70.0.0

python -c 'import setuptools; print(setuptools.__version__); from pkg_resources import packaging; print("SUCCESS")'
70.0.0
<string>:1: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: cannot import name 'packaging' from 'pkg_resources' (/home/dxia/.pyenv/versions/prolific-fm/lib/python3.10/site-packages/pkg_resources/__init__.py)

Code of Conduct

  • I agree to follow the PSF Code of Conduct
@davidxia davidxia added documentation Needs Triage Issues that need to be evaluated for severity and status. labels May 21, 2024
@abravalheri
Copy link
Contributor

abravalheri commented May 21, 2024

Hi @davidxia, thank you for opening this issue, but I am afraid I disagree with the following conclusion:

69.5.1 had packaging.pkg_resources. I can run from pkg_resources import packaging.
...
Should this breaking change be stated in the changelog?

You can see that pkg_resources does contain __all__ and that packaging is not listed there in v69.5.1:

https://github.com/pypa/setuptools/blob/v69.5.1/pkg_resources/__init__.py#L190-L273

i.e. pkg_resources is not a provider of packaging.

This is a clear indicator that users should not expect an stable behaviour if they decide to run from pkg_resources import packaging. (Probably linters, editors/IDEs might show warnings in this context; as well as auto-complete might not work).

The fact that users were trying to run from pkg_resources import packaging, means that they were relying on a side-effect/implementation detail of pkg_resources, and that is not guaranteed to be stable between releases.

So my conclusion is that this is not a breaking change in terms of "API promises".

@abravalheri
Copy link
Contributor

abravalheri commented May 21, 2024

This is how the Python docs describe __all__:

The public names defined by a module are determined by checking the module’s namespace for a variable named __all__; if defined, it must be a sequence of strings which are names defined or imported by that module. The names given in __all__ are all considered public and are required to exist. If __all__ is not defined, the set of public names includes all names found in the module’s namespace which do not begin with an underscore character ('_'). __all__ should contain the entire public API. It is intended to avoid accidentally exporting items that are not part of the API (such as library modules which were imported and used within the module).

This is also further explained in PEP8:

Imported names should always be considered an implementation detail. Other modules must not rely on indirect access to such imported names unless they are an explicitly documented part of the containing module’s API, such as os.path or a package’s __init__ module that exposes functionality from submodules.

@davidxia
Copy link
Author

Gotcha thanks for the explanation

@jeffhataws
Copy link

This is breaking PyTorch 2.1 (not PyTorch 2.2+). Will you consider adding it back?

(aws_neuron_venv_pytorch) ubuntu@ip-10-0-8-190:~$ python -c "from torch.utils import cpp_extension"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/ubuntu/aws_neuron_venv_pytorch/lib/python3.8/site-packages/torch/utils/cpp_extension.py", line 28, in <module>
    from pkg_resources import packaging  # type: ignore[attr-defined]
ImportError: cannot import name 'packaging' from 'pkg_resources' (/home/ubuntu/aws_neuron_venv_pytorch/lib/python3.8/site-packages/pkg_resources/__init__.py)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
Development

No branches or pull requests

3 participants