Skip to content

Improve for loop index variable type narrowing - #18014

Merged
JukkaL merged 2 commits into
python:masterfrom
cdce8p:improve-index-typing
Oct 23, 2024
Merged

Improve for loop index variable type narrowing#18014
JukkaL merged 2 commits into
python:masterfrom
cdce8p:improve-index-typing

Conversation

@cdce8p

Copy link
Copy Markdown
Collaborator

Preserve the literal type of index expressions a bit longer (until the next assignment) to support TypedDict lookups.

fromtypingimportTypedDictclassX(TypedDict):
hourly: intdaily: intdeffunc(x: X) ->None:
forvarin ("hourly", "daily"):
print(x[var])

Closes#9230

@github-actions

This comment has been minimized.

@cdce8p

Copy link
Copy Markdown
CollaboratorAuthor

Mypy primer results

scrapy

+ scrapy/downloadermiddlewares/cookies.py:145: error: "str" has no attribute "decode"; maybe "encode"? [attr-defined]+ scrapy/downloadermiddlewares/cookies.py:152: error: "str" has no attribute "decode"; maybe "encode"? [attr-defined]

These are true positives. All TypedDict values str or bool. The intention is probably to apply decode to extra bytes items. Annotating it would require PEP 728.

https://github.com/scrapy/scrapy/blob/b4bad97eae6bcce790a626d244c63589f4134408/scrapy/downloadermiddlewares/cookies.py#L141-L145

@JukkaLJukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice to see several now-unnecessary type ignores in mypy_primer output! Looks good, just one minor test-related suggestion.

a = "yearly"
reveal_type(a) # N: Revealed type is "builtins.str"
a = 1 # E: Incompatible types in assignment (expression has type "int", variable has type "str")
reveal_type(a) # N: Revealed type is "builtins.str"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Test b = a followed by reveal_type(b).

@github-actions

Copy link
Copy Markdown
Contributor

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

pydantic (https://github.com/pydantic/pydantic)
+ pydantic/v1/networks.py:345: error: Unused "type: ignore" comment [unused-ignore]
scrapy (https://github.com/scrapy/scrapy)
+ scrapy/downloadermiddlewares/cookies.py:141: error: Unused "type: ignore" comment [unused-ignore]+ scrapy/downloadermiddlewares/cookies.py:142: error: Unused "type: ignore" comment [unused-ignore]+ scrapy/downloadermiddlewares/cookies.py:145: error: Unused "type: ignore" comment [unused-ignore]+ scrapy/downloadermiddlewares/cookies.py:145: error: "str" has no attribute "decode"; maybe "encode"? [attr-defined]+ scrapy/downloadermiddlewares/cookies.py:145: note: Error code "attr-defined" not covered by "type: ignore" comment+ scrapy/downloadermiddlewares/cookies.py:152: error: Unused "type: ignore" comment [unused-ignore]+ scrapy/downloadermiddlewares/cookies.py:152: error: "str" has no attribute "decode"; maybe "encode"? [attr-defined]+ scrapy/downloadermiddlewares/cookies.py:152: note: Error code "attr-defined" not covered by "type: ignore" comment
discord.py (https://github.com/Rapptz/discord.py)
- discord/message.py:2265: error: TypedDict key must be a string literal; expected one of ("channel_id", "guild_id", "id", "author", "content", ...) [literal-required]
core (https://github.com/home-assistant/core)
+ homeassistant/components/image_processing/__init__.py:226: error: Unused "type: ignore" comment [unused-ignore]+ homeassistant/components/energy/data.py:334: error: Unused "type: ignore" comment [unused-ignore]
schemathesis (https://github.com/schemathesis/schemathesis)
+ src/schemathesis/cli/__init__.py:260: error: Unused "type: ignore" comment [unused-ignore]

@JukkaL
JukkaL merged commit 9e68959 into python:masterOct 23, 2024
@cdce8p
cdce8p deleted the improve-index-typing branch October 23, 2024 11:59
JukkaL pushed a commit that referenced this pull request May 29, 2026
Fixes#19317 Uses exsiting index narrowing in for-loop. Previous PR:
#18014
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Looping through literals not typed correctly

2 participants

@cdce8p@JukkaL