GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Adopt "more sound" type annotations by abravalheri · Pull Request #487 · python/importlib_metadata

Conversation

@abravalheri

@abravalheri abravalheri commented May 2, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Motivated by #486.

This PR just provides more realistic type annotations1 to improve the type checking soundness of the package.

It does not solve #486: all the limitations are still present, but at least type checkers would help to notify users when they are trying to use new APIs that are not safe (not necessary all the time, but better than now).

It is a bit of a pain, but these annotations should be a bit more sound than they currently are.

This should also prevent future versions of this library from introducing incompatible behaviour (thanks to pytest-mypy).

Footnotes

  1. Technically speaking, "a bit more correct". Because importlib_metadata collects 3rd-party implementations via find_distributions everything can happen. ↩

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@abravalheri abravalheri changed the title Adopt more realistic type annotations Adopt "more sound" type annotations May 2, 2024
@abravalheri
abravalheri marked this pull request as ready for review May 2, 2024 10:44
@jaraco

jaraco commented Jun 23, 2024

Copy link
Copy Markdown
Member

Thanks for the proposal. I'm going to have to think about this more.

This library is merged back into CPython as importlib.metadata so these annotations won't make sense when that happens. Moreover, this is the first introduction of references to importlib.metadata in this library, so I'm unsure what the implications will be.

Is there any way to achieve the objectives either by:

  • adjusting the patch to be universal (so it's meaningful in both importilb_metadata and importlib.metadata), or
  • moving the backport-specific concerns to a ./compat/* module that would be omitted downstream in CPython?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

abravalheri added 4 commits June 24, 2024 09:47
@abravalheri
Use more realistic return types
3a90a8d
@abravalheri
Use type check guard for legacy annotations
c1612ec
@abravalheri
Fix type annotation for Python 3.10+
e30d8f7
@abravalheri
Fix warnings in sphinx docs
2044db1
@abravalheri
abravalheri force-pushed the issue-486 branch from 99ef61a to 77eb71e Compare June 24, 2024 09:33
@abravalheri
Move code specific to backport to compat module
4fe96a4
@abravalheri
abravalheri force-pushed the issue-486 branch from 77eb71e to 4fe96a4 Compare June 24, 2024 09:41
@abravalheri

abravalheri commented Jun 24, 2024

Copy link
Copy Markdown
Contributor Author

Thanks @jaraco for the code review.

Is there any way to achieve the objectives either by:

  • adjusting the patch to be universal (so it's meaningful in both importilb_metadata and importlib.metadata), or

Unfortunately I don't think so.
importlib_metadata has a backwards compatibility problem that simply disappears when it becomes importlib.metadata.

In this sense only importlib_metadata need these contrived type annotations to inform end-users that they can get back legacy objects when using the API.

  • moving the backport-specific concerns to a ./compat/* module that would be omitted downstream in CPython?

I gave it a try... don't know if that is exactly what you had in mind, but I tried to do my best.
Please note that the annotations are not specific to a version of Python, but always will be necessary while importlib_metadata exists in PyPI. In the future I believe they may be useful during development (e.g. by identifying places where legacy objects are retrieved and preventing incompatible methods to be called on them).

I am not good with names, so if you have suggestions regarding names please let me know.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

abravalheri
abravalheri commented Jun 24, 2024
View reviewed changes
Comment thread importlib_metadata/compat/stdlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@abravalheri
Workaround sphinx limitations
c86976d
@jaraco jaraco mentioned this pull request Jun 24, 2024
API incompatibility with importlib.metadata (or at least the API is not type-safe?) #486
Closed
jaraco
jaraco reviewed Sep 11, 2024
View reviewed changes
Comment thread importlib_metadata/compat/stdlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@jaraco jaraco mentioned this pull request Sep 11, 2024
Enforce local versions of objects and introduce shared protocols #505
Merged
@jaraco

jaraco commented Nov 22, 2025

Copy link
Copy Markdown
Member

As discussed in #486, I believe the direction we're going is to unify the the interfaces (through #505 plus new protocols).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jaraco jaraco closed this Nov 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaraco jaraco jaraco left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@abravalheri @jaraco
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.