Support incompatible Python types in x-python-type extension#2841
Support incompatible Python types in x-python-type extension#2841
Conversation
📝 WalkthroughWalkthroughThis PR adds x-python-type compatibility handling to the JSON Schema parser: it detects when x-python-type conflicts with schema types, provides DataType overrides with appropriate Import objects, and integrates this into data-type and object-field resolution. Changes
Sequence Diagram(s)(silently omitted — not applicable) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (8)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-12-25T09:22:14.661ZApplied to files:
📚 Learning: 2025-12-18T13:43:16.235ZApplied to files:
🧬 Code graph analysis (7)tests/data/expected/main/jsonschema/x_python_type_fqpath.py (2)
tests/data/expected/main/jsonschema/x_python_type_callable_anyof.py (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
tests/main/jsonschema/test_main_jsonschema.py (2)
tests/data/expected/main/jsonschema/x_python_type_callable.py (1)
tests/data/expected/main/jsonschema/x_python_type_custom_fqpath.py (5)
tests/data/expected/main/jsonschema/x_python_type_compatible_set.py (6)
🪛 Ruff (0.14.10)src/datamodel_code_generator/parser/jsonschema.py1319-1319: Unused Remove unused (RUF100) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
🔇 Additional comments (11)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
📚 Docs Preview: https://pr-2841.datamodel-code-generator.pages.dev |
CodSpeed Performance ReportMerging #2841 will not alter performanceComparing
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2841 +/- ##
=======================================
Coverage 99.49% 99.49%
=======================================
Files 90 90
Lines 14290 14336 +46
Branches 1705 1713 +8
=======================================
+ Hits 14218 14264 +46
Misses 37 37
Partials 35 35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
559-571: Consider documenting the coverage scope ofPYTHON_TYPE_IMPORTS.The mapping covers
collections.abcandremodule types appropriately. This appears to focus on types that require non-root imports.For clarity, consider adding a docstring or comment explaining the scope:
- Why certain types are included (collections.abc, re.Pattern)
- Why typing module types (List, Dict, Optional, etc.) are not included
- Guidance for extending this mapping if needed
This would help future maintainers understand the design decision.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/datamodel_code_generator/parser/jsonschema.pytests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
src/datamodel_code_generator/imports.py (2)
Import(20-38)from_full_path(35-38)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/parser/jsonschema.py
1319-1319: Unused noqa directive (non-enabled: PLR6301)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.10 on Windows
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: 3.11 on Windows
- GitHub Check: py312-pydantic1 on Ubuntu
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.13 on macOS
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (5)
tests/main/jsonschema/test_main_jsonschema.py (1)
6970-7082: New x-python-type tests look solid and align with the intended semanticsThe added tests exercise the key scenarios for
x-python-typeoverrides (direct property, anyOf/nullable, compatible container override viaSet[str], fully-qualifiedcollections.abc.Callable, and missingtypefalling back toAny). Usage ofgeneratewith an in-memory JSON string andInputFileType.JsonSchemamatches the documented module API, and the assertions are precise enough without being brittle against formatting. I don’t see any functional or style issues here.src/datamodel_code_generator/parser/jsonschema.py (4)
49-49: LGTM! Import addition is appropriate.The
Importclass is correctly imported and will be used by the newPYTHON_TYPE_IMPORTSmapping and_get_python_type_overridemethod.
1186-1188: LGTM! Clean integration with early return pattern.The override check is appropriately placed at the beginning of
get_data_type, following the guard clause pattern. This ensures that incompatiblex-python-typeextensions take precedence over standard type resolution.
537-557: TheCOMPATIBLE_PYTHON_TYPESmapping is complete and correctly designed. It includes all concrete container types that represent valid JSON array/object alternatives: Iterable, Iterator, and Generator are intentionally excluded because they are abstract protocols and lazy-evaluation types that don't semantically map to JSON array containers. These types remain available inPYTHON_TYPE_IMPORTSfor use as incompatible overrides (e.g., when a field should beCallableor aGenerator), which is the correct design pattern.
1327-1349: The code appears sound. The edge cases raised in the original comment are actually handled correctly by design:
Malformed type annotations (e.g.,
"List["): The_get_python_type_base()method correctly extracts the base type usingsplit("[", maxsplit=1)[0], so"List["yields"List"without error.Missing imports (
import_=None): This is intentional. TheDataType.import_field defaults toNone(line 330 in types.py:import_: Optional[Import] = None), which is valid and expected. Only the 11 stdlib types inPYTHON_TYPE_IMPORTS(Callable, Iterable, Iterator, Generator, Awaitable, Coroutine, AsyncIterable, AsyncIterator, AsyncGenerator, Pattern, Match) get explicit import entries. Custom types like"MyCustomType"correctly receiveimport_=None, making users responsible for providing their own imports—a reasonable design choice.The compatibility checking and override logic is sound and properly tested.
koxudaxi
left a comment
koxudaxi
left a comment
There was a problem hiding this comment.
The # noqa: PLR6301 directive is actually needed. When removed, the linter triggers PLR6301 error: "Method _get_python_type_base could be a function, class method, or static method". The directive correctly suppresses this warning since we want to keep it as an instance method for consistency with other methods in the class.
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR adds support for incompatible Python types in the x-python-type extension. Previously, when x-python-type specified a type incompatible with the JSON schema type (e.g., Callable with type: string), the extension was silently ignored and the schema type was used. Now, the x-python-type value is respected even for incompatible types. This is a feature enhancement rather than a breaking change because: (1) users who specified incompatible x-python-type values clearly wanted that type to be used, so honoring it is the expected behavior; (2) compatible types (Set, Mapping, Sequence, etc. with matching schema types) continue to work exactly as before through the existing _get_python_type_flags mechanism; (3) no existing valid use of x-python-type is affected negatively - only previously-ignored configurations now work as intended. This analysis was performed by Claude Code Action |
|
🎉 Released in 0.51.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.