refactor(python): use built-in type hinting generics #22925
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 477 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="samples/openapi3/client/petstore/python-aiohttp/petstore_api/api_client.py">
<violation number="1" location="samples/openapi3/client/petstore/python-aiohttp/petstore_api/api_client.py:444">
P2: Deserializer now only recognizes lowercase `list[`/`dict[` strings, but response type maps still emit `List[...]`/`Dict[...]`, causing AttributeError or incorrect deserialization.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+444
to
456
| if klass.startswith('list['): | ||
| m = re.match(r'list\[(.*)]', klass) | ||
| assert m is not None, "Malformed list type definition" | ||
| sub_kls = m.group(1) | ||
| return [self.__deserialize(sub_data, sub_kls) | ||
| for sub_data in data] | ||
|
|
||
| if klass.startswith('Dict['): | ||
| m = re.match(r'Dict\[([^,]*), (.*)]', klass) | ||
| assert m is not None, "Malformed Dict type definition" | ||
| if klass.startswith('dict['): | ||
| m = re.match(r'dict\[([^,]*), (.*)]', klass) | ||
| assert m is not None, "Malformed dict type definition" | ||
| sub_kls = m.group(2) | ||
| return {k: self.__deserialize(v, sub_kls) | ||
| for k, v in data.items()} |
Contributor
There was a problem hiding this comment.
P2: Deserializer now only recognizes lowercase list[/dict[ strings, but response type maps still emit List[...]/Dict[...], causing AttributeError or incorrect deserialization.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/python-aiohttp/petstore_api/api_client.py, line 444:
<comment>Deserializer now only recognizes lowercase `list[`/`dict[` strings, but response type maps still emit `List[...]`/`Dict[...]`, causing AttributeError or incorrect deserialization.</comment>
<file context>
@@ -441,16 +441,16 @@ def __deserialize(self, data, klass):
- if klass.startswith('List['):
- m = re.match(r'List\[(.*)]', klass)
- assert m is not None, "Malformed List type definition"
+ if klass.startswith('list['):
+ m = re.match(r'list\[(.*)]', klass)
+ assert m is not None, "Malformed list type definition"
</file context>
Suggested change
| if klass.startswith('list['): | |
| m = re.match(r'list\[(.*)]', klass) | |
| assert m is not None, "Malformed list type definition" | |
| sub_kls = m.group(1) | |
| return [self.__deserialize(sub_data, sub_kls) | |
| for sub_data in data] | |
| if klass.startswith('Dict['): | |
| m = re.match(r'Dict\[([^,]*), (.*)]', klass) | |
| assert m is not None, "Malformed Dict type definition" | |
| if klass.startswith('dict['): | |
| m = re.match(r'dict\[([^,]*), (.*)]', klass) | |
| assert m is not None, "Malformed dict type definition" | |
| sub_kls = m.group(2) | |
| return {k: self.__deserialize(v, sub_kls) | |
| for k, v in data.items()} | |
| if klass.startswith(('list[', 'List[')): | |
| m = re.match(r'[lL]ist\[(.*)]', klass) | |
| assert m is not None, "Malformed list type definition" | |
| sub_kls = m.group(1) | |
| return [self.__deserialize(sub_data, sub_kls) | |
| for sub_data in data] | |
| if klass.startswith(('dict[', 'Dict[')): | |
| m = re.match(r'[dD]ict\[([^,]*), (.*)]', klass) | |
| assert m is not None, "Malformed dict type definition" | |
| sub_kls = m.group(2) | |
| return {k: self.__deserialize(v, sub_kls) | |
| for k, v in data.items()} |
Contributor
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="samples/openapi3/client/petstore/python-pydantic-v1/tests/test_deserialization.py">
<violation number="1" location="samples/openapi3/client/petstore/python-pydantic-v1/tests/test_deserialization.py:78">
P2: ApiClient.__deserialize only handles "List[...]" and "Dict[...]" strings, so the new lowercase built-in generic strings like "dict[str, Pet]"/"list[Pet]" will not be recognized and will fail deserialization unless the implementation is updated.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -75,13 +75,13 @@ def test_deserialize_dict_str_pet(self): | |||
| } | |||
| response = MockResponse(data=json.dumps(data)) | |||
|
|
|||
| deserialized = self.deserialize(response, 'Dict[str, Pet]') | |||
| deserialized = self.deserialize(response, 'dict[str, Pet]') | |||
Contributor
There was a problem hiding this comment.
P2: ApiClient.__deserialize only handles "List[...]" and "Dict[...]" strings, so the new lowercase built-in generic strings like "dict[str, Pet]"/"list[Pet]" will not be recognized and will fail deserialization unless the implementation is updated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/client/petstore/python-pydantic-v1/tests/test_deserialization.py, line 78:
<comment>ApiClient.__deserialize only handles "List[...]" and "Dict[...]" strings, so the new lowercase built-in generic strings like "dict[str, Pet]"/"list[Pet]" will not be recognized and will fail deserialization unless the implementation is updated.</comment>
<file context>
@@ -75,13 +75,13 @@ def test_deserialize_dict_str_pet(self):
response = MockResponse(data=json.dumps(data))
- deserialized = self.deserialize(response, 'Dict[str, Pet]')
+ deserialized = self.deserialize(response, 'dict[str, Pet]')
self.assertTrue(isinstance(deserialized, dict))
self.assertTrue(isinstance(deserialized['pet'], petstore_api.Pet))
</file context>
Contributor
Author
|
There are still a few references to |
Contributor
There was a problem hiding this comment.
3 issues found across 902 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache:15">
P2: Using `list[str]` without `from __future__ import annotations` breaks generated code on Python <3.9, but the python‑aiohttp templates still declare support for Python 3.6+ (and even 3.4/3.5). This is a compatibility regression.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/python-flask/base_model.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/python-flask/base_model.mustache:13">
P2: Using built-in generics (dict[str, …], type[T]) requires Python 3.9+, but the python-flask templates still target Python 3.6/3.4/3.5. This change will raise TypeError on import in older supported versions, making it a compatibility regression unless the minimum Python version is bumped or annotations are postponed.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/python-pydantic-v1/model_generic.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/python-pydantic-v1/model_generic.mustache:33">
P2: Using built-in generics (`dict[str, Any]`) in Pydantic v1 models will break on supported Python 3.7/3.8 runtimes, since Pydantic evaluates annotations at runtime and built-in generics are only subscriptable in Python 3.9+.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
|
|
||
| def validate_scope_{{name}}(required_scopes: List[str], token_scopes: List[str]) -> bool: | ||
| def validate_scope_{{name}}(required_scopes: list[str], token_scopes: list[str]) -> bool: |
Contributor
There was a problem hiding this comment.
P2: Using list[str] without from __future__ import annotations breaks generated code on Python <3.9, but the python‑aiohttp templates still declare support for Python 3.6+ (and even 3.4/3.5). This is a compatibility regression.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/python-aiohttp/security_controller.mustache, line 15:
<comment>Using `list[str]` without `from __future__ import annotations` breaks generated code on Python <3.9, but the python‑aiohttp templates still declare support for Python 3.6+ (and even 3.4/3.5). This is a compatibility regression.</comment>
<file context>
@@ -14,7 +12,7 @@ def info_from_{{name}}(token: str) -> dict:
-def validate_scope_{{name}}(required_scopes: List[str], token_scopes: List[str]) -> bool:
+def validate_scope_{{name}}(required_scopes: list[str], token_scopes: list[str]) -> bool:
""" Validate required scopes are included in token scope """
return set(required_scopes).issubset(set(token_scopes))
</file context>
| # openapiTypes: The key is attribute name and the | ||
| # value is attribute type. | ||
| openapi_types: typing.Dict[str, type] = {} | ||
| openapi_types: dict[str, type] = {} |
Contributor
There was a problem hiding this comment.
P2: Using built-in generics (dict[str, …], type[T]) requires Python 3.9+, but the python-flask templates still target Python 3.6/3.4/3.5. This change will raise TypeError on import in older supported versions, making it a compatibility regression unless the minimum Python version is bumped or annotations are postponed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/python-flask/base_model.mustache, line 13:
<comment>Using built-in generics (dict[str, …], type[T]) requires Python 3.9+, but the python-flask templates still target Python 3.6/3.4/3.5. This change will raise TypeError on import in older supported versions, making it a compatibility regression unless the minimum Python version is bumped or annotations are postponed.</comment>
<file context>
@@ -10,14 +10,14 @@ T = typing.TypeVar('T')
# openapiTypes: The key is attribute name and the
# value is attribute type.
- openapi_types: typing.Dict[str, type] = {}
+ openapi_types: dict[str, type] = {}
# attributeMap: The key is attribute name and the
</file context>
| {{/vars}} | ||
| {{#isAdditionalPropertiesTrue}} | ||
| additional_properties: Dict[str, Any] = {} | ||
| additional_properties: dict[str, Any] = {} |
Contributor
There was a problem hiding this comment.
P2: Using built-in generics (dict[str, Any]) in Pydantic v1 models will break on supported Python 3.7/3.8 runtimes, since Pydantic evaluates annotations at runtime and built-in generics are only subscriptable in Python 3.9+.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/python-pydantic-v1/model_generic.mustache, line 33:
<comment>Using built-in generics (`dict[str, Any]`) in Pydantic v1 models will break on supported Python 3.7/3.8 runtimes, since Pydantic evaluates annotations at runtime and built-in generics are only subscriptable in Python 3.9+.</comment>
<file context>
@@ -30,7 +30,7 @@ class {{classname}}({{#parent}}{{{.}}}{{/parent}}{{^parent}}BaseModel{{/parent}}
{{/vars}}
{{#isAdditionalPropertiesTrue}}
- additional_properties: Dict[str, Any] = {}
+ additional_properties: dict[str, Any] = {}
{{/isAdditionalPropertiesTrue}}
__properties = [{{#allVars}}"{{baseName}}"{{^-last}}, {{/-last}}{{/allVars}}]
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
these are supported from 3.9 onwards, matching the current python constraint, so non-breaking
https://stackoverflow.com/questions/37087457/difference-between-defining-typing-dict-and-dict#37087556
https://peps.python.org/pep-0585/
Summary by cubic
Refactor all Python generators and samples to use built-in generics (dict/list/tuple/set) per PEP 585. Simplifies type hints, drops extra typing imports and redundant mappings, and keeps Python 3.9+ compatibility with no behavior changes.
Written for commit bfbf615. Summary will update on new commits.