Repository navigation
refactor(python): use built-in type hinting generics - #22925
timonrieger wants to merge 1 commit into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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.
| 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()} |
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>
| 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()} |
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]') | |||
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>
|
There are still a few references to |
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: |
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] = {} |
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] = {} |
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>
|
@wing328 i applied this to all python generators, but since some are still supporting python 3.8 and below this would be breaking. should we focus this PR on the python generator (not sure how simple this will be since the java code is shared) or point this to the breaking v8 branch? cc @cbornet (2017/09) @tomplus (2018/10) @krjakbrjak (2023/02) @fa0311 (2023/10) @multani (2023/10) |
Python 3.8 is EOL for over a year now. |
bfbf615 to
e9fef9d
Compare
c73ce99 to
55062af
Compare
55062af to
5c077cb
Compare
|
closing as ruff will apply these changes anyways on a |
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 Python generators, templates, and samples to use built-in generics (dict/list/tuple/set) per PEP 585, and harden codegen to prevent schema/container keywords from leaking into model imports. No behavior changes; Python 3.9+ only.
python,pydantic-v1,fastapi,flask,aiohttp,blueplanet): replacedtyping.Dict/List/Tuple/Setwithdict/list/tuple/set, updatedClassVar[...]andtype[T], removed unusedtypingimports, and adjusted docstrings.RequestSerialized, response/header maps, and configTypedDicts use built-ins; ApiClient deserializers now matchlist[anddict[; asyncio REST kwargs/maps usedict[str, Any]; FastAPI base API usesClassVar[tuple]; security scope helpers expectlist[str].List/Dictprimitives and per-generatorarray/map/setoverrides; centralized mappings in abstract Python codegens; added guards so container names and schema keywords don’t enter model imports; tests updated to assert built-ins only.Dict/Listfrom language primitives; regenerated docs, tests, and sample clients to use built-ins.Written for commit 5c077cb. Summary will update on new commits.