fix(http-client-python): generate valid Python type annotations in types.py - #11638
fix(http-client-python): generate valid Python type annotations in types.py#11638Libba Lawrence (l0lawrence) wants to merge 4 commits into
Conversation
…pes.py Fixes three codegen bugs producing invalid annotations in generated types.py: - Internal enums used as TypedDict fields are imported as a bare symbol from their private _enums submodule, and the annotation no longer emits an undefined _enums. prefix. - Deduplicate runtime and TYPE_CHECKING imports of the same symbol to avoid mypy no-redef errors. - Emit a flat (non-inheriting) TypedDict when a child overrides an inherited field's requiredness, satisfying PEP 589. Adds regression tests for all three cases. Fixes: #11626 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a8cce88-1663-4cc6-a634-2e83e35d04b7
commit: |
|
All changed packages have been documented.
Show changes
|
Python emitter diffBaseline No changes to generated output. Rendered diff: inline on the run summary, or the emitter-diff-html artifact. Informational check (eng/emitter-diff); does not block the PR. |
|
You can try these changes here
|
These tokens appear in the http-client-python import dedup comments/code and in the changelog entry that references the mypy 'no-redef' error code. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a8cce88-1663-4cc6-a634-2e83e35d04b7
| } | ||
| if not regular_bound_names: | ||
| return | ||
| self.file_import.imports = [ |
There was a problem hiding this comment.
which things do we keep in typing imports only? is it just the token credential from azure.core?
There was a problem hiding this comment.
Things we keep typing-only today are the imports that are only referenced inside quoted/forward-ref annotations and must not be imported at runtime, e.g.:
TokenCredential/AsyncTokenCredential+SdkCoreTypewithisTypingOnly(primitive_types.py)- forward-referenced model/enum annotations (
model_type.py,enum_type.py) and a few response/parameter/combined-type cases
The only case this should touch and remove from type-checking is the same symbol imported both at runtime (from its _enums submodule) and again under TYPE_CHECKING (from the public models package)
Resolve conflict in tests/unit/test_typeddict.py (combine import blocks) and update Bug 1/Bug 3 regression fixtures to set is_typed_dict_only so models render under main's new is_used_in_operations_via_types gating (#11639). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a8cce88-1663-4cc6-a634-2e83e35d04b7
There was a problem hiding this comment.
Pull request overview
Fixes multiple Python emitter codegen issues that produced invalid type annotations in generated types.py files (reported for azure-search-documents), primarily impacting MyPy correctness for internal enums, duplicate imports, and TypedDict inheritance requiredness.
Changes:
- Emit consistent internal-enum annotations and imports for
types.py(use bare forward refs and import from private_enumswhen needed). - Deduplicate
TYPE_CHECKINGimports when the same bound name is already imported at runtime to avoid MyPyno-redef. - Emit “flat” (non-inheriting) TypedDicts when a child changes an inherited field’s requiredness (PEP 589 compliance), with unit test coverage for all three regressions.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/http-client-python/tests/unit/test_typeddict.py | Adds regression tests covering internal enum import/reference, import dedupe, and flat TypedDict requiredness override. |
| packages/http-client-python/generator/pygen/codegen/serializers/types_serializer.py | Introduces needs_flat_typeddict and uses it to avoid illegal TypedDict inheritance and unnecessary parent imports. |
| packages/http-client-python/generator/pygen/codegen/serializers/import_serializer.py | Adds dedupe pass to remove redundant TYPE_CHECKING imports that would cause MyPy no-redef. |
| packages/http-client-python/generator/pygen/codegen/models/enum_type.py | Adjusts enum annotation/import behavior for types.py, including internal enums importing from private _enums. |
| cspell.yaml | Adds “dedupe” and “redef” to dictionary for new terminology used in comments/strings. |
| .chronus/changes/fix-python-emitter-invalid-type-annotations-2026-4-18-0-0-0.md | Adds changelog entry describing the three bug fixes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if self.internal: | ||
| enums_module = ( | ||
| f"{relative_path}models.{self.code_model.enums_filename}" | ||
| if relative_path != "." | ||
| else f".models.{self.code_model.enums_filename}" | ||
| ) | ||
| else: | ||
| enums_module = f"{relative_path}models" if relative_path != "." else ".models" |
…module_name Address Copilot review: get_relative_import_path can return values like '..indexes' (no trailing dot), so f-string concatenation produced invalid paths such as '..indexesmodels._enums'. Use get_relative_import_path(..., module_name=...) so the dot is inserted correctly for both internal and public enums. Adds cross-namespace regression tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a8cce88-1663-4cc6-a634-2e83e35d04b7
The Python client emitter generated
types.pyfiles with invalid type annotations forazure-search-documents, producing 4 MyPy errors across three generated files. This fixes the three distinct codegen bugs behind them. Each bug below shows the generated-code output before and after the fix.Bug 1: inconsistent internal-enum export and reference
azure/search/documents/types.pyBefore, the import pulled the bare name from the public
.modelspackage, where the internal enum is not re-exported (Module "...models" has no attribute "SemanticQueryRewritesResultType" [attr-defined]), and the annotation referenced an undefined_enums.prefix (Name "_enums" is not defined [name-defined]). After, the enum symbol is imported directly from the private._enumssubmodule and the annotation uses the matching bare forward reference. Enums stay private (no change to the public__all__).Bug 2: duplicate enum import
azure/search/documents/knowledgebases/types.pyBefore, the same symbol was imported at runtime from its
_enumssubmodule and again underif TYPE_CHECKING:from the public package (Name "KnowledgeSourceKind" already defined [no-redef]). After, a dedup pass drops theTYPE_CHECKINGduplicate whose bound name is already imported at runtime; the runtime import is sufficient for the annotations.Bug 3: TypedDict requiredness override via inheritance
azure/search/documents/indexes/types.pyclass SearchIndexerKnowledgeStoreProjectionSelector(TypedDict, total=False): referenceKeyName: str generatedKeyName: str - class SearchIndexerKnowledgeStoreTableProjectionSelector( - SearchIndexerKnowledgeStoreProjectionSelector - ): + class SearchIndexerKnowledgeStoreTableProjectionSelector(TypedDict, total=False): + referenceKeyName: str generatedKeyName: Required[str] tableName: Required[str]Before, the child subclassed the parent and redeclared
generatedKeyNameasRequired[str], which PEP 589 forbids (Overwriting TypedDict field "generatedKeyName" while extending [misc]). After, the child renders as a flat, non-inheritingTypedDictthat lists every field (inherited plus own) directly, so requiredness is expressed without illegal inheritance.Notes for reviewers
_enumsrather than the_enumsmodule, which also avoids name collisions when internal enums come from several sibling namespaces.tests/unit/test_typeddict.pycovering all three cases.Validation
test_typeddict.py+test_enums.py: 50 passed (42 existing + 8 new).azure/search/documentspackage built from the buggy forms reproduces theattr-defined,name-defined, andmisc(TypedDict) errors undermypy --python-version 3.10, and the fixed forms type-check with exit 0. Bug 2's duplicate import is confirmed removed at the serializer level (baseline emits two imports ofKnowledgeSourceKind, the fix emits one).Fixes: #11626