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

[BUG] Setuptools overriding standard lib importlib.metadata.PathDistribution #3169

Closed
tlambert03 opened this issue Mar 13, 2022 · 7 comments
Closed
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@tlambert03
Copy link

setuptools version

60.9.3

Python version

python 3.9

OS

macOS

Additional environment information

❯ pip list
Package    Version
---------- -------
pip        22.0.4
setuptools 60.9.3
wheel      0.37.1

Description

this seems similar to #3102, but issue was closed by #3108, without activity since then, and I still seeing this issue in the latest release.

importing setuptools overrides/removes the behavior and return type of of the standard library importlib.metadata.distribution() function:

Expected behavior

importing setuptools doesn't supersede/remove the distribution finder from standard library importlib

How to Reproduce

  1. import setuptools
  2. call importlib.metadata.distribution()

Output

>>> from importlib import metadata
>>> metadata.distribution('pip')  # returns the expected type
<importlib.metadata.PathDistribution object at 0x101665910>
>>> import sys
>>> [getattr(finder, 'find_distributions', None) for finder in sys.meta_path]
[None, None, None, <bound method PathFinder.find_distributions of <class '_frozen_importlib_external.PathFinder'>>]

# importing setuptools changes the type of object returned by metadata.distribution
# and removes the _frozen_importlib_external.PathFinder from 
# the list of finders used by `metadata.Distribution.discover`
>>> import setuptools
>>> metadata.distribution('pip')
<setuptools._vendor.importlib_metadata.PathDistribution object at 0x102253070>
>>> [getattr(finder, 'find_distributions', None) for finder in sys.meta_path]
[None, None, None, None, None, None, <bound method MetadataPathFinder.find_distributions of <setuptools._vendor.importlib_metadata.MetadataPathFinder object at 0x101fdad00>>]

# note:
>>> import importlib_metadata
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'importlib_metadata'
@tlambert03 tlambert03 added bug Needs Triage Issues that need to be evaluated for severity and status. labels Mar 13, 2022
@tlambert03
Copy link
Author

tlambert03 commented Mar 13, 2022

fwiw, git bisect puts this at 2d6cc80

(before that the following script would exit successfully)

from importlib import metadata
assert isinstance(metadata.distribution('pip'), metadata.Distribution)
import setuptools
assert isinstance(metadata.distribution('pip'), metadata.Distribution)

(but it's possible i didn't setup the setuptools dev environment properly and that's a red herring 😄 )

@abravalheri
Copy link
Contributor

abravalheri commented Mar 13, 2022

Hi @tlambert03 thank you very much for the in-depth study about this behaviour.

Do you have a use case that depends on the particular implementation of the PathDistribution?

I was going through the importlib.metadata docs and it seems to me that extended search algorithms and custom finders in the meta path are part of the documented API.

Is setuptools._vendor.importlib_metadata.PathDistribution causing other troubles down the line?

@tlambert03
Copy link
Author

tlambert03 commented Mar 13, 2022

Thanks @abravalheri, I found this issue while working on the plugin architecture for a program that I work on (napari).

I have a bit of code that checks isinstance(obj, importlib.metadata.Distribution), and I found it was suddenly failing in my recently built environments. While I personally don't import setuptools anywhere, I have to assume one of my dependencies tries to import it.

From a typing perspective, my code was sound. For example if you run the following script through mypy:

from importlib import metadata
d = metadata.distribution('pip')
reveal_type(d)

the output will be:

❯ mypy script.py
script.py:3:13: note: Revealed type is "importlib.metadata.Distribution"

however, because setuptools._vendor.importlib_metadata.PathDistribution is not a sublcass of importlib.metadata.Distribution, it breaks that typing (and my program).

One could certainly make the argument that I should only be doing structural typing, and should not have used isinstance, (and indeed I will adjust my code to do that in the future).

I was going through the importlib.metadata docs and it seems to me that extended search algorithms and custom finders in the meta path are part of the documented API.

I agree! definitely fine to add stuff to meta_path: I guess more than anything, I'm wondering why the _frozen_importlib_external.PathFinder had to be removed from sys.meta_path and why setuptools._vendor.importlib_metadata.PathDistribution couldn't have just been added to the end?

@abravalheri
Copy link
Contributor

Thanks for the quick response @tlambert03.

I'm wondering why the _frozen_importlib_external.PathFinder had to be removed from sys.meta_path and why setuptools._vendor.importlib_metadata.PathDistribution couldn't have just been added to the end?

According to #3102, the versions of importlib.metadata between 3.7 and 3.10 are incompatible between themselves (until very recently the importlib.metadata was considered a provisional module). The change in the meta_path seems to be a workaround to prevent other errors happening due to this inconsistency.

A very similar behaviour will also be observed if at any point the user of your program installs a package inside the same virtual environment that uses the importlib_metadata backport:

https://github.com/python/importlib_metadata/blob/ee566d048c0061b4f846f100ebfd93eefbcbf608/importlib_metadata/_compat.py#L27

I think the best here is to do a structural check, since setuptools is not the only dependency that can break your plugin architecture.

@tlambert03
Copy link
Author

I think the best here is to do a structural check, since setuptools is not the only dependency that can break your plugin architecture.

I agree. thanks for having a look. If you feel like this is just how it's going to be, I can close this issue (but feel free to re-open if you think there's anything actionable here on your end)

@abravalheri
Copy link
Contributor

I think it would be nice to avoid breaking the users' expectations like it is happening right now, but since this workaround comes directly from importlib_metadata, there might be very little we can do in setuptools 😓

This workaround also seems necessary for importlib_metadata to work properly.

@tlambert03
Copy link
Author

you have a nearly impossible task here at setuptools :) keep up the good work

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

No branches or pull requests

2 participants