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

Sphinx v8 compatibility: separate the names of two mutually-shadowing variables whose types have diverged #14

Merged
merged 1 commit into from
Jul 25, 2024
Merged

Sphinx v8 compatibility: separate the names of two mutually-shadowing variables whose types have diverged #14

merged 1 commit into from
Jul 25, 2024

Conversation

jayaddison
Copy link
Contributor

@jayaddison jayaddison commented Jul 25, 2024

The values of these key variables would have clobbered each other in the past; their value is not used in subsequent code, however, so this value-overwriting behaviour does not have any impact on runtime behaviour.

Prior to Sphinx v8, both of them had type str, and so mypy did not detect a problem. With Sphinx v8 their types diverge as a result of sphinx-doc/sphinx#12575 - one of them remains str and the other becomes str | None.

By assigning each of them a distinct variable name, this changeset resolves mypy validation errors when the Sphinx v8.0.0 codebase is installed as a type-checking dependency (discovered during #13).

cc @AA-Turner

@AA-Turner AA-Turner merged commit 1425734 into sphinx-doc:master Jul 25, 2024
8 checks passed
@AA-Turner
Copy link
Member

Thanks!

A

@jayaddison jayaddison deleted the fixup/unshadow-diverging-key-type-variables branch July 26, 2024 09:25
@jayaddison
Copy link
Contributor Author

Thank you!

AA-Turner pushed a commit that referenced this pull request Jul 27, 2024
AA-Turner pushed a commit that referenced this pull request Jul 27, 2024
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.

2 participants