Support --use-annotated *and* --use-non-positive-negative-number-constrained-types#3015
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an internal mapping of specialized numeric constrained types to the JSON Schema keys they consume; removes those consumed keys from field constraints when emitting Annotated fields; refines how numeric constraints are collected and passed to data-type constructors; and adds tests plus expected output for annotated + constrained-type combinations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Merging this PR will not alter performance
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/datamodel_code_generator/parser/jsonschema.py`:
- Around line 1309-1311: Remove the commented-out debug statement causing Ruff
ERA001: delete the line "sys.stderr.write(f\"yoyo {kwargs}\\n\\n\")" from the
block where kwargs = model_dump(obj) and the subsequent if
self.field_constraints check; if you still need runtime debug info, replace it
with a proper logger.debug call (e.g., use the module/class logger) rather than
a commented or inline sys.stderr write so linting and CI pass for jsonschema.py
around model_dump and self.field_constraints.
- Around line 1320-1325: The current strict check requiring len(number_kwargs)
== 1 prevents emitting NonNegative*/NonPositive* types when one of the numeric
bounds equals 0 but additional numeric constraints exist; locate the logic in
jsonschema.py around number_kwargs, kwargs_to_pass and
data_type_manager.use_non_positive_negative_number_constrained_types and replace
the single-constraint gate with a check that detects a zero bound (e.g. check if
0 is present in number_kwargs.values() or specifically if
minimum/exclusiveMinimum/maximum/exclusiveMaximum == 0) and, when
use_non_positive_negative_number_constrained_types is true, set kwargs_to_pass =
number_kwargs so the specialized NonNegative*/NonPositive* types are emitted
while leftover numeric constraints remain on Field().
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3015 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 94 94
Lines 18152 18176 +24
Branches 2101 2108 +7
=========================================
+ Hits 18152 18176 +24
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:
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
1317-1321:⚠️ Potential issue | 🟠 MajorRelax the single-constraint gate so zero-bound specialized types still emit with extra numeric constraints.
On Line 1317,
len(number_kwargs) == 1is too strict. Cases likeminimum=0plus another numeric constraint won’t emitNonNegative*/NonPositive*, even though the extra constraints can remain onField(...).Suggested fix
- # Only works if there is exactly one number constraint - if len(number_kwargs) == 1: - v = next(iter(number_kwargs.values())) - if v == 0 and self.data_type_manager.use_non_positive_negative_number_constrained_types: - kwargs_to_pass = number_kwargs + if self.data_type_manager.use_non_positive_negative_number_constrained_types: + zero_bound_keys = [k for k, v in number_kwargs.items() if v == 0] + if len(zero_bound_keys) == 1: + k = zero_bound_keys[0] + kwargs_to_pass = {k: number_kwargs[k]}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/parser/jsonschema.py` around lines 1317 - 1321, The code currently only enables zero-bound specialized types when number_kwargs has exactly one entry; relax this to detect any numeric constraint equal to 0 so NonNegative*/NonPositive* types still emit even with extra constraints. In the block referencing number_kwargs, kwargs_to_pass, and self.data_type_manager.use_non_positive_negative_number_constrained_types, replace the len(number_kwargs) == 1 check with a condition that checks whether any value in number_kwargs equals 0 (e.g., any(v == 0 for v in number_kwargs.values())) and keep setting kwargs_to_pass = number_kwargs when true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/datamodel_code_generator/parser/jsonschema.py`:
- Around line 1317-1321: The code currently only enables zero-bound specialized
types when number_kwargs has exactly one entry; relax this to detect any numeric
constraint equal to 0 so NonNegative*/NonPositive* types still emit even with
extra constraints. In the block referencing number_kwargs, kwargs_to_pass, and
self.data_type_manager.use_non_positive_negative_number_constrained_types,
replace the len(number_kwargs) == 1 check with a condition that checks whether
any value in number_kwargs equals 0 (e.g., any(v == 0 for v in
number_kwargs.values())) and keep setting kwargs_to_pass = number_kwargs when
true.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/datamodel_code_generator/parser/jsonschema.py (1)
1320-1325:⚠️ Potential issue | 🟠 MajorLine 1321 gating is still too strict for mixed numeric constraints.
Requiring
len(number_kwargs) == 1preventsNonNegative*/NonPositive*from being used when a zero-bound exists alongside another numeric bound (for example,minimum=0withmaximum=10), even though the extra constraint can remain inField(...).Suggested fix
- # Only works if there is exactly one number constraint - if len(number_kwargs) == 1: - v = next(iter(number_kwargs.values())) - if v == 0 and self.data_type_manager.use_non_positive_negative_number_constrained_types: - kwargs_to_pass = number_kwargs + if self.data_type_manager.use_non_positive_negative_number_constrained_types: + zero_bound_keys = [k for k, v in number_kwargs.items() if v == 0] + if len(zero_bound_keys) == 1: + key = zero_bound_keys[0] + # Pass only the zero-bound key so the type manager can pick + # NonNegative*/NonPositive* while remaining constraints stay on Field(...) + kwargs_to_pass = {key: number_kwargs[key]}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/parser/jsonschema.py` around lines 1320 - 1325, The guard requiring len(number_kwargs) == 1 is too strict; change the logic in the JSON Schema parser so that if self.data_type_manager.use_non_positive_negative_number_constrained_types is true and any numeric bound in number_kwargs has the zero value (i.e., any(v == 0 for v in number_kwargs.values())), set kwargs_to_pass to a dict containing only that zero-bound (extract the key/value where v == 0) so the NonNegative*/NonPositive* typed choice can be used while leaving the remaining numeric constraints in number_kwargs to be emitted later (keep references to number_kwargs, kwargs_to_pass, and self.data_type_manager.use_non_positive_negative_number_constrained_types).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/datamodel_code_generator/parser/jsonschema.py`:
- Around line 1320-1325: The guard requiring len(number_kwargs) == 1 is too
strict; change the logic in the JSON Schema parser so that if
self.data_type_manager.use_non_positive_negative_number_constrained_types is
true and any numeric bound in number_kwargs has the zero value (i.e., any(v == 0
for v in number_kwargs.values())), set kwargs_to_pass to a dict containing only
that zero-bound (extract the key/value where v == 0) so the
NonNegative*/NonPositive* typed choice can be used while leaving the remaining
numeric constraints in number_kwargs to be emitted later (keep references to
number_kwargs, kwargs_to_pass, and
self.data_type_manager.use_non_positive_negative_number_constrained_types).
koxudaxi
force-pushed
the
support-use-annotated-with-non-positive
branch
from
dc72fac to
0133e16
Compare
koxudaxi
force-pushed
the
support-use-annotated-with-non-positive
branch
from
0133e16 to
3475626
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_main_kr.py`:
- Around line 1388-1415: Add the same Pydantic-version guard as the neighboring
tests: decorate
test_use_non_positive_negative_number_constrained_types_with_other_constraints_with
`@pytest.mark.skipif`(pydantic.VERSION < "2.0.0", reason="requires pydantic>=2.0")
so the test is skipped for Pydantic < 2.0.0; ensure pytest and pydantic are
imported at the top if not already, and match the exact decorator style used on
the adjacent tests.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/use_non_positive_negative_with_other_constraints.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (3)
src/datamodel_code_generator/parser/jsonschema.pytests/data/expected/main_kr/use_non_positive_negative_with_other_constraints/output.pytests/test_main_kr.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/data/expected/main_kr/use_non_positive_negative_with_other_constraints/output.py
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR is a bug fix that enables the existing This analysis was performed by Claude Code Action |
|
🎉 Released in 0.54.1 This PR is now available in the latest release. See the release notes for details. |
3 participants
Fixes #3014
Summary by CodeRabbit
Bug Fixes
New Features
Tests