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

Add deprecation classes in importlib.metadata #11118

Merged
merged 3 commits into from
Dec 10, 2023
Merged

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Dec 7, 2023

related to #3968

Generally speaking these classes print warnings and then call super().method() without changing anything's type signature, so they don't need much to be filled out here.

@tungol tungol marked this pull request as draft December 7, 2023 19:57

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Dec 8, 2023

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@tungol tungol marked this pull request as ready for review December 8, 2023 00:59
Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM, but one question.

if sys.version_info >= (3, 11):
def __lt__(self, other: object) -> bool: ...
if sys.version_info < (3, 12):
def __iter__(self) -> Iterator[Any]: ... # result of iter((str, Self)), really
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why not use Iterator[tuple[str, Self]]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an iterator that returns tuple[str, Self]. This is an iterator that will always return two items, the first of which is str and the second of which is Self. As far as I know, there's no way to express that in the current system. We could do Iterator[str | Self], but my understanding is that it's better to avoid union return types.

The implementation here is return iter((self.name, self)); I meant "iter()` as the runtime function, not a misspelling of the type, but that's a bit confusing because then I switched to the types of the values. Maybe the comment could be better worded, but I'm not sure how without getting too wordy. Might be better to just leave off the comment if it's too confusing. The function was removed in 3.12, so this is going away (eventually) anyway.

@srittau srittau merged commit 695d67c into python:main Dec 10, 2023
67 checks passed
@tungol tungol deleted the importlib branch December 10, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants