Final purge of direct pkg_resources usages #10675
Conversation
uranusjr
added
the
skip news
uranusjr
force-pushed
the
metadata-misc
branch
2 times, most recently
from
a3a79b1 to
174babb
Compare
uranusjr
force-pushed
the
metadata-misc
branch
2 times, most recently
from
20893bf to
794347d
Compare
|
Should be ready. This changes quite several files, but is mostly just moving things into |
Relevant functionalities are moved into pip._internal.metadata.
uranusjr
force-pushed
the
metadata-misc
branch
from
794347d to
496343e
Compare
|
Yay, everything seems to work! |
|
That's a great achievement. Thanks for this effort! |
| @@ -149,17 +148,17 @@ def __init__(self, *, package: str, reason: str) -> None: | |||
|
|
|||
|
|
|||
| class NoneMetadataError(PipError): | |||
There was a problem hiding this comment.
For a follow up, we should probably give this a more meaningful name -- DeclaredMetadataFileIsMissingError or something similar.
There was a problem hiding this comment.
FWIW this exception never actually makes sense; it’s caught in exactly two places, one just sets the result to an empty string, and the other is the outmost except PipError block that prints the exception as an error message. It will probably go away entirely when the importlib.metadata backend gets implemented.
There was a problem hiding this comment.
Well, if I remember correctly, the situation which this was originally raised in was something weird, like a metadata file that has a truth has_metadata but get_metadata returned None. I'm not quite sure how this could happen, TBH. I do remember seeing this exception get raised in another situation in one of these porting PRs, which might have changed the meaning of it?
If we're sure that this will never happen with importlib metadata, then yea, this can probably go away.
There was a problem hiding this comment.
the situation which this was originally raised in was something weird, like a metadata file that has a truth has_metadata but get_metadata returned None. I'm not quite sure how this could happen, TBH.
That’s correct, and the exception can happen for permission reasons. But the thing is that this discrepency isn’t really necessary due to how the exception is handled. And this also does not happen in importlib.metadata because has_metadata does not even exist in the API—the only way to know if a metadata file exists in importlib.metadata is try to read it.
| except UnsupportedWheel as e: | ||
| raise UnsupportedWheel(f"{name} has an invalid wheel, {e}") |
There was a problem hiding this comment.
Why not let this error pass through as-is?
The caller has the name and can use it, if they really want to, no?
There was a problem hiding this comment.
Yes, but I don’t really want to think about changing behaviour right now; I’ll get too many chances for that when we try to re-implement everything in importlib.metadata and deal with those subtle differences.
I think this is it! No more pkg_resources calls (except from the compatibility shim)!
Needs to first fix #10674. Submit to run CI first…Done and ready