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

Replace Any in optparse #11599

Merged
merged 6 commits into from
Mar 15, 2024
Merged

Replace Any in optparse #11599

merged 6 commits into from
Mar 15, 2024

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Mar 14, 2024

The first commit replaces some Anys with the correct type and should probably be reviewed in isolation.

The second commit replaces all remaining Anys with Incomplete (or removes the annotation completely). This may be overzealous, as there might be legitimate Anys, but I opted for the "safer" side, as I don't have time to review them and they are all a bit suspect.

This comment has been minimized.

@srittau
Copy link
Collaborator Author

srittau commented Mar 14, 2024

Regarding the primer output: The pip annotations need to be updated to include None. The code already handles the None case correctly.

This comment has been minimized.

@Avasam
Copy link
Collaborator

Avasam commented Mar 14, 2024

This PR works towards #9550
(This is not a review, just crosslinking)

Copy link
Contributor

@hamdanal hamdanal left a comment

Choose a reason for hiding this comment

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

I worked with optparse internals recently and I got somewhat familiar with it so I decided to leave you some suggestions that I thought could help your PR :)

stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
stdlib/optparse.pyi Outdated Show resolved Hide resolved
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pip (https://github.com/pypa/pip)
+ src/pip/_internal/cli/parser.py:70: error: Argument 1 of "format_description" is incompatible with supertype "HelpFormatter"; supertype defines the argument type as "str | None"  [override]
+ src/pip/_internal/cli/parser.py:70: note: This violates the Liskov substitution principle
+ src/pip/_internal/cli/parser.py:70: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
+ src/pip/_internal/cli/parser.py:88: error: Argument 1 of "format_epilog" is incompatible with supertype "HelpFormatter"; supertype defines the argument type as "str | None"  [override]
+ src/pip/_internal/cli/parser.py:88: note: This violates the Liskov substitution principle
+ src/pip/_internal/cli/parser.py:88: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides

Copy link
Contributor

@hamdanal hamdanal left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

Both mypy primer hits in pip are true positives. Their functions accept None but are annotated as accepting only str (see here); I guess they were just following the old optparse annotations.

@srittau srittau merged commit 9841c25 into python:main Mar 15, 2024
55 checks passed
@srittau srittau deleted the optparse-any branch March 15, 2024 21:33
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