Conversation
WalkthroughThis PR introduces a new CLI option Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2756 +/- ##
=======================================
Coverage 99.47% 99.47%
=======================================
Files 83 83
Lines 12457 12474 +17
Branches 1487 1490 +3
=======================================
+ Hits 12391 12408 +17
Misses 35 35
Partials 31 31
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:
|
CodSpeed Performance ReportMerging #2756 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/datamodel_code_generator/cli_options.py (1)
171-172: Doc metadata for the new option is correctly added.The
CLI_OPTION_METAentry for--use-tuple-for-fixed-itemsunderOptionCategory.TYPINGkeeps CLI docs in sync with argparse. Once you cut a release, you may want to fill insince_versionfor nicer docs, but it’s not required for correctness.src/datamodel_code_generator/parser/graphql.py (1)
159-160: GraphQL parser wiring is fine; note that the flag is currently a no-op here.Accepting and forwarding
use_tuple_for_fixed_itemskeeps parser signatures uniform, and there’s no functional issue. Since this flag only has meaning for JSON Schema–style “items array” schemas, it currently has no effect for GraphQL inputs; consider calling that out in docs if you expect users to combine the option with GraphQL sources.Also applies to: 263-264
tests/main/jsonschema/test_main_jsonschema.py (1)
3366-3396: Newitems_array_tupletest is consistent with existing patternsThe decorators, CLI doc metadata, and
run_main_and_assertcall mirror the existingprefix_itemstests and correctly exercise--use-tuple-for-fixed-itemswithmin_version/pydantic v2. I don’t see functional issues here. If you want the CLI docs to hint at neighboring options, you could optionally addrelated_options(e.g. collection flags) to the@pytest.mark.cli_docblock, but it’s not required for correctness.src/datamodel_code_generator/parser/jsonschema.py (1)
900-906: Consider explicit None handling for clarity.The logic correctly detects fixed-length tuples, but the chained comparison
obj.minItems == obj.maxItems == len(...)relies on implicit None comparison behavior. While functionally correct (returns False when either is None), explicit None checks would improve readability and make the intent clearer.🔎 Optional refactor for clarity
def _is_fixed_length_tuple(self, obj: JsonSchemaObject) -> bool: """Check if an array field represents a fixed-length tuple.""" if obj.prefixItems is not None and obj.items in {None, False}: - return obj.minItems == obj.maxItems == len(obj.prefixItems) + if obj.minItems is None or obj.maxItems is None: + return False + return obj.minItems == obj.maxItems == len(obj.prefixItems) if self.use_tuple_for_fixed_items and isinstance(obj.items, list) and obj.prefixItems is None: - return obj.minItems == obj.maxItems == len(obj.items) + if obj.minItems is None or obj.maxItems is None: + return False + return obj.minItems == obj.maxItems == len(obj.items) return False
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/items_array_tuple.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (13)
docs/cli-reference/index.mddocs/cli-reference/quick-reference.mddocs/cli-reference/typing-customization.mdsrc/datamodel_code_generator/__init__.pysrc/datamodel_code_generator/__main__.pysrc/datamodel_code_generator/arguments.pysrc/datamodel_code_generator/cli_options.pysrc/datamodel_code_generator/parser/base.pysrc/datamodel_code_generator/parser/graphql.pysrc/datamodel_code_generator/parser/jsonschema.pysrc/datamodel_code_generator/parser/openapi.pytests/data/expected/main/jsonschema/items_array_tuple.pytests/main/jsonschema/test_main_jsonschema.py
⏰ 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). (9)
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.10 on macOS
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (14)
docs/cli-reference/typing-customization.md (2)
24-24: LGTM!The table entry is properly formatted and provides a clear, concise description of the new option.
3088-3139: Excellent documentation for the new feature!The section is well-structured and provides:
- Clear description of when tuples are generated
- Practical usage example
- Complete input/output demonstration showing the transformation from a fixed-length array to
tuple[float, float]The example effectively demonstrates the core use case mentioned in issue #2247.
docs/cli-reference/index.md (1)
12-12: LGTM!The documentation updates correctly:
- Increment the Typing Customization options count from 19 to 20
- Add the new option to the alphabetical index with proper linking
Also applies to: 187-187
src/datamodel_code_generator/__main__.py (2)
436-436: LGTM!The configuration option is properly declared with:
- Appropriate type annotation (
bool)- Sensible default value (
False) for an opt-in feature- Logical placement near related array/collection options (
use_unique_items_as_set)
746-746: LGTM!The parameter is correctly passed from the config to the
generate()function, following the established pattern used for other similar options.docs/cli-reference/quick-reference.md (1)
46-46: LGTM!The quick reference documentation correctly adds the new option:
- In the Typing Customization section with proper description and link
- In the alphabetical index at the correct position
Both entries are consistent with the existing format and link to the detailed documentation.
Also applies to: 287-287
src/datamodel_code_generator/parser/base.py (1)
736-737: Newuse_tuple_for_fixed_itemsflag is wired cleanly; please double‑check interaction with--use-unique-items-as-set.The new constructor parameter and instance attribute are consistent with neighboring options and correctly exposed on
Parser.One thing to verify:
__replace_unique_list_to_set()still converts list‑typed fields withconstraints.unique_itemsintosetwhenuse_unique_items_as_setis enabled. For fixed‑items arrays that you now map to tuples, ensure those cases either:
- don’t set
constraints.unique_items, or- no longer present as
is_listat this stage,so that
--use-tuple-for-fixed-itemsand--use-unique-items-as-setcan’t silently fight over the same field.Also applies to: 846-847
tests/data/expected/main/jsonschema/items_array_tuple.py (1)
1-11: Expected output for tuple-typed field looks correct.The generated model header and
point: tuple[float, float]annotation align with the new flag’s intent and existing expected-output style.src/datamodel_code_generator/parser/openapi.py (1)
243-244: OpenAPI parser correctly propagatesuse_tuple_for_fixed_items.The new flag is added to
OpenAPIParser.__init__and forwarded in thesuper().__init__call in the same relative position as in the base parser, so OpenAPI component schemas and inlined schemas will honor the tuple-for-fixed-items behavior without additional changes here.Also applies to: 347-348
src/datamodel_code_generator/arguments.py (1)
489-494: CLI flag definition is consistent with existing typing options.
--use-tuple-for-fixed-itemsfollows the established pattern (store_true, default=None under “Typing customization”), so it should integrate cleanly with config loading and thegenerate()API.src/datamodel_code_generator/__init__.py (1)
469-469:use_tuple_for_fixed_itemsthreading throughgenerate()looks correctAdding
use_tuple_for_fixed_itemsas a defaulted kwarg and forwarding it into the parser constructor is consistent with how other feature flags are wired, and keeps existinggenerate()callers source-compatible.Also applies to: 724-724
src/datamodel_code_generator/parser/jsonschema.py (3)
583-583: LGTM! Parameter addition looks good.The new
use_tuple_for_fixed_itemsparameter is properly added to the constructor with a sensible default value (False) and correctly forwarded to the parent class initialization.Also applies to: 686-686
1059-1061: LGTM! Constraint suppression is correctly implemented.Suppressing
minItemsandmaxItemsconstraints for fixed-length tuples is the correct behavior, as the tuple length is already determined by the number of elements in the tuple definition.
2448-2454: LGTM! Array parsing integration is correct.The helper method is properly integrated into
parse_array_fieldsto handle both:
- Legacy approach:
itemsas a list (when flag is enabled)- Modern approach:
prefixItems(JSON Schema 2020-12 standard)The
is_tupleandsuppress_item_constraintsflags are set consistently in both cases.
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR adds a new opt-in feature via the This analysis was performed by Claude Code Action |
Fixes: #2247
Summary by CodeRabbit
Release Notes
New Features
--use-tuple-for-fixed-itemsCLI option to generate tuple types for arrays with fixed-length items arrays.Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.