Use a dict to keep track of TypedDict fields in semanal #18369
Conversation
for more information, see https://pre-commit.ci
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Overall I think the change makes sense. Some points
- Do we (or plugin authors) at any point need to access the field_name / type by index? I guess one of the reasons for list was that dicts didn't keep the insertion order in the past. It should probably be possible still do index access with
list(fields)[i]instead now. - This might be a breaking change for plugin authors. Don't know of any but I remember that I once experimented with a custom plugin using the
TypedDictAnalyzer.
IIRC the separate name and type lists are a common pattern in the mypy code base. Especially for function arguments. I did wonder in the past if there is a better way to do it as those are often iterated over together but I guess we probably shouldn't break that much working code unless we have to.
| def analyze_typeddict_classdef_fields( | ||
| self, defn: ClassDef, oldfields: list[str] | None = None | ||
| ) -> tuple[list[str] | None, list[Type], list[Statement], set[str], set[str]]: | ||
| self, defn: ClassDef, oldfields: Collection[str] | None = None | ||
| ) -> tuple[dict[str, Type] | None, list[Statement], set[str], set[str]]: |
There was a problem hiding this comment.
The docstring here should be updated.
|
I think hopefully not? TypedDictType already uses a dictionary and TypedDictAnalyzer currently complains about duplicate keys. My motivation here is making #7435 easier in some future PR. I think this is maybe category 2 or 3 of plugin related change. Searching online in Github, I could find just one instance of breakage here https://github.com/linw1995/data_extractor , but I think that one is already not up to date for the introduction of ReadOnly |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Useful for #7435