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

chore: update dependencies #8984

Merged
merged 1 commit into from
Feb 25, 2024

Conversation

radoering
Copy link
Member

No description provided.

@dimbleby
Copy link
Contributor

fyi I have jaraco/keyring#666 and jaraco/jaraco.classes#13 open trying to fix up the faulty types in those two projects. I wouldn't wait on them though: in poetry it only makes a difference to type annotations in unit test so certainly not worth holding up a release

Not sure about the changes in site_packages.py, if importlib-metadata doesn't promise to return a Path then I'd have thought the code was fine as it was?

(find_distribution_files_with_suffix() seems to be unused so that could be removed altogether, that's one less to worry about...)

@radoering
Copy link
Member Author

Not sure about the changes in site_packages.py, if importlib-metadata doesn't promise to return a Path then I'd have thought the code was fine as it was?

importlib-metadata promises to return a SimplePath, which is a Protocol that just provides a subset of Path. According to mypy we cannnot create a Path from a SimplePath (that's probably true). Thus, the old code will only work if we get a real Path - or strictly speaking a PathLike or str. I assume it's fine to expect a real path since we are working with a venv. Or do you think it would be better to keep the old code and just add a #type: ignore?

@dimbleby
Copy link
Contributor

Ah, different python versions - this returns Pathlike[str] according to typeshed.

I'd think the right thing to do is to raise a pull request at importlib-metadata, changing the return type to be a plain Path.

Presumably they are currently trying to avoid committing to a particular type, but hopefully should be open to agreeing that in fact returning their own odd half-Path is unhelpful for callers.

(Meanwhile if you are looking to cut a release... do whatever)

@dimbleby
Copy link
Contributor

dimbleby commented Feb 18, 2024

ugh, though python/importlib_metadata#480

in fact could be a zipfile.Path or other SimplePath

but I think this over-shot: it makes sense for the abstract Distribution to commit only to an abstract Path-ish, but a PathDistribution really should return a concrete Path. Maybe.

At the very least: SimplePath is not even exported by importlib-metadata so as things stand it really is hard for a type-conscious codebase to use this API.

@radoering radoering requested a review from a team February 18, 2024 16:42
@radoering radoering force-pushed the update-dependencies branch 3 times, most recently from bcb8710 to d20a7c3 Compare February 24, 2024 21:46
@radoering radoering enabled auto-merge (squash) February 25, 2024 06:34
@radoering radoering merged commit cc32ce6 into python-poetry:master Feb 25, 2024
20 checks passed
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants