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

Complete type annotations in pip/_internal/commands #10182

Merged
merged 5 commits into from Jul 23, 2021

Conversation

harupy
Copy link
Contributor

@harupy harupy commented Jul 23, 2021

Complete type annotations in pip/_internal/commands.

@@ -18,7 +18,7 @@
# in a test-related module).
# Finally, we need to pass an iterable of pairs here rather than a dict
# so that the ordering won't be lost when using Python 2.7.
commands_dict = OrderedDict([
commands_dict: "OrderedDict[str, CommandInfo]" = OrderedDict([
Copy link
Member

@uranusjr uranusjr Jul 23, 2021

Choose a reason for hiding this comment

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

You need to jump through some hoops for this to work on 3.6.

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing import OrderedDict
else:
    from collections import OrderedDict

commands_dict: "OrderedDict[str, CommandInfo]" = OrderedDict(...)

But does this have to be an ordered dict? Usually it's good enough to just annotate this as dict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But does this have to be an ordered dict? Usually it's good enough to just annotate this as dict.

Agree, I'll change OrderedDict to Dict.

Copy link
Contributor Author

@harupy harupy Jul 23, 2021

Choose a reason for hiding this comment

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

@uranusjr
10b35b6 This is what you meant, right?

Copy link
Member

Choose a reason for hiding this comment

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

Yup 👍

Copy link
Member

@pradyunsg pradyunsg Jul 23, 2021

Choose a reason for hiding this comment

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

FWIW, given that we're Python 3.6+, we can also just change this into a dictionary; since those are ordered starting Python 3.6.

But that can be done separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, thanks for the suggestion!

@uranusjr uranusjr merged commit 2de3af1 into pypa:main Jul 23, 2021
@harupy harupy deleted the type-annotations-commands branch July 23, 2021 10:43
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2021
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.

None yet

3 participants