Support ClassVar for Pydantic v2#2920
Conversation
📝 WalkthroughWalkthroughAdd ClassVar support for Pydantic v2 generation by importing Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
![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)
tests/main/jsonschema/test_main_jsonschema.py (1)
7908-7917: Clarify test intent and double‑check necessity of--field-include-all-keys.The test wiring (input/expected paths and extra args) looks correct for exercising the new Pydantic v2 ClassVar behavior via
has_classvar_extra.json/has_classvar_extra.py. Two minor points:
- The docstring
"Test default value handling."is misleading here and duplicates an earlier test; consider updating it to explicitly mention ClassVar/x‑is_classvar handling for Pydantic v2, e.g."Test ClassVar handling from x-is_classvar extra for Pydantic v2.".- The use of
--field-include-all-keysis fine, but ifx-is_classvaris intended to work even when that flag is not provided, it might be worth either:
- Dropping the flag here, or
- Adding a second test without
--field-include-all-keysto lock in that behavior (and documenting if the flag is in fact required for this feature).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/datamodel_code_generator/model/template/pydantic_v2/BaseModel.jinja2is excluded by none and included by nonetests/data/jsonschema/has_classvar_extra.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (3)
src/datamodel_code_generator/model/pydantic_v2/base_model.pytests/data/expected/main/jsonschema/has_classvar_extra.pytests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T08:25:19.839Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2890
File: tests/data/expected/main/jsonschema/ref_nullable_with_constraint.py:14-15
Timestamp: 2026-01-02T08:25:19.839Z
Learning: The datamodel-code-generator currently generates RootModel subclasses with an explicit `root` field annotation (e.g., `class StringType(RootModel[str]): root: str`). This is existing behavior of the code generator and should not be flagged as an issue introduced by new changes.
Applied to files:
tests/data/expected/main/jsonschema/has_classvar_extra.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.py
🧬 Code graph analysis (2)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (1)
src/datamodel_code_generator/model/base.py (3)
imports(326-351)imports(812-817)type_hint(278-296)
tests/main/jsonschema/test_main_jsonschema.py (2)
tests/main/conftest.py (2)
output_file(99-101)run_main_and_assert(245-409)tests/test_main_kr.py (1)
output_file(44-46)
🔇 Additional comments (4)
src/datamodel_code_generator/model/pydantic_v2/base_model.py (4)
34-34: LGTM! Import added appropriately.The IMPORT_CLASSVAR import is correctly placed and follows the established pattern for conditional imports.
195-197: LGTM! ClassVar detection correctly implemented.The property appropriately checks for the
x-is_classvarextra with a strictis Truecomparison, which aligns with the PR objectives and issue #2919.
191-193: Property implementation is correct, but template usage cannot be verified in the sandbox environment.The
class_var_type_hintproperty correctly returnsf"ClassVar[{self.type_hint}]". The supporting infrastructure (is_class_varproperty andIMPORT_CLASSVARimport handling) is in place. However, the referenced template file (pydantic_v2/BaseModel.jinja2) is not accessible in the sandbox, so template usage cannot be directly confirmed.
199-214: The implementation correctly handles ClassVar imports and doesn't wrap ClassVar fields with Field(). The code is sound:
- When
is_class_varis True, IMPORT_CLASSVAR is properly added (lines 204-205)- IMPORT_FIELD is only appended for discriminators (line 211), not for ClassVar fields
- The expected output confirms ClassVar fields are not wrapped with Field():
namespace: ClassVar[str | None] = 'test'- Test coverage exists via
test_jsonschema_classvar_extra_pydantic_v2The original concerns about Field() wrapping ClassVar and missing test coverage are not valid—the code and tests already handle these cases correctly.
koxudaxi
left a comment
koxudaxi
left a comment
There was a problem hiding this comment.
Thank you for craeting the new PR.
I left few comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @src/datamodel_code_generator/model/pydantic/base_model.py:
- Around line 265-272: Remove the leftover commented-out debug code inside the
type_hint property: delete the two commented lines "# if self.name == \"name\":"
and "# breakpoint()" within the ModelPydanticBaseModel.type_hint property so
the method simply returns ClassVar[...] when self.is_class_var otherwise
delegates to super().type_hint; leave the rest of the method intact.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/has_classvar_extra.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (2)
src/datamodel_code_generator/model/pydantic/base_model.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/datamodel_code_generator/model/pydantic_v2/base_model.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T08:25:19.839Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2890
File: tests/data/expected/main/jsonschema/ref_nullable_with_constraint.py:14-15
Timestamp: 2026-01-02T08:25:19.839Z
Learning: The datamodel-code-generator currently generates RootModel subclasses with an explicit `root` field annotation (e.g., `class StringType(RootModel[str]): root: str`). This is existing behavior of the code generator and should not be flagged as an issue introduced by new changes.
Applied to files:
src/datamodel_code_generator/model/pydantic/base_model.py
🧬 Code graph analysis (1)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
src/datamodel_code_generator/model/base.py (3)
repr_set_sorted(84-94)type_hint(278-296)annotated(422-424)
🔇 Additional comments (4)
src/datamodel_code_generator/model/pydantic/base_model.py (4)
22-22: LGTM: Import supports consistent set representation.The
repr_set_sortedimport enables deterministic set representations for both regular and ClassVar default values, which is necessary for code generation consistency.
261-263: LGTM: Clean implementation of ClassVar detection.The
is_class_varproperty correctly checks thex-is-classvarcustom JSON Schema extension, aligning with the solution proposed in issue #2919.
279-280: LGTM: Correct handling of ClassVar with Annotated.The modification correctly avoids wrapping
ClassVarfields inAnnotated, since ClassVar fields use simple assignment rather thanField()construction. This ensures the generated code isname: ClassVar[Type] = valueinstead of an invalidAnnotatedwrapper.
256-257: Handle the case where ClassVar has no default value.When
self.defaultisUNDEFINED, returningrepr(UNDEFINED)generates invalid code like<object object at 0x...>. ClassVar fields without a default should produce a bare type annotation (name: ClassVar[Type]), not an assignment.🔎 Proposed fix
if self.is_class_var: + if self.default is UNDEFINED: + return "" return repr_set_sorted(self.default) if isinstance(self.default, set) else repr(self.default)Note: Current test cases only cover ClassVar with defaults. Verify whether ClassVar fields without defaults are a supported use case to determine if this is an active bug or defensive programming for an unsupported scenario.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
284-292: Add ClassVar import handling to the imports property.The
type_hintproperty returnsClassVar[...]whenis_class_varis True (line 271), but theimportsproperty doesn't add the requiredIMPORT_CLASSVARimport. The pydantic_v2 implementation correctly handles this at lines 196–197. Update the imports method to include a check foris_class_varand appendIMPORT_CLASSVARwhen needed, similar to how it handlesIMPORT_FIELD.
🧹 Nitpick comments (1)
tests/data/expected/main/jsonschema/has_classvar_extra.py (1)
9-9: Consider conditionally importing Field.Field is imported but not used in the generated code. Since ClassVar fields don't require Field(), the generator could optimize by only importing Field when it's actually needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/has_classvar_extra.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (4)
src/datamodel_code_generator/model/pydantic/base_model.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.pytests/data/expected/main/jsonschema/has_classvar_extra.pytests/main/jsonschema/test_main_jsonschema.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/main/jsonschema/test_main_jsonschema.py
- src/datamodel_code_generator/model/pydantic_v2/base_model.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-02T08:25:19.839Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2890
File: tests/data/expected/main/jsonschema/ref_nullable_with_constraint.py:14-15
Timestamp: 2026-01-02T08:25:19.839Z
Learning: The datamodel-code-generator currently generates RootModel subclasses with an explicit `root` field annotation (e.g., `class StringType(RootModel[str]): root: str`). This is existing behavior of the code generator and should not be flagged as an issue introduced by new changes.
Applied to files:
tests/data/expected/main/jsonschema/has_classvar_extra.pysrc/datamodel_code_generator/model/pydantic/base_model.py
🧬 Code graph analysis (1)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
src/datamodel_code_generator/model/base.py (3)
repr_set_sorted(84-94)type_hint(278-296)annotated(422-424)
🔇 Additional comments (6)
tests/data/expected/main/jsonschema/has_classvar_extra.py (1)
12-13: LGTM! ClassVar field correctly generated.The ClassVar annotation and direct default assignment match Pydantic's expectations for class-level variables.
src/datamodel_code_generator/model/pydantic/base_model.py (5)
22-22: Import addition is correct.The repr_set_sorted import is properly sourced and used for consistent set representation in default values.
256-258: ClassVar field serialization is correct.Returning just the default representation for ClassVar fields is appropriate since they don't use Field(). This correctly generates direct assignment syntax like
namespace: ClassVar[str] = 'test'.
265-272: Type hint wrapping logic is correct.The property correctly wraps the parent type hint with
ClassVar[...]whenis_class_varis True, producing the expected type annotations likeClassVar[str | None].
279-280: Annotated handling for ClassVar is correct.Returning just
self.type_hintfor ClassVar fields is appropriate since they don't use Field() and shouldn't be wrapped in Annotated[].
261-263: No action needed. The property name convention is correct and consistent.The code checks for
"x-is-classvar"(with hyphen) in line 263, and the actual JSON Schema test input (tests/data/jsonschema/has_classvar_extra.json) also uses"x-is-classvar"with hyphen. This naming convention is consistent across the codebase—other x- extensions like"x-is-base-class"also use the hyphen format, which follows the JSON Schema convention for vendor extensions.
|
I rebased my branch |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodSpeed Performance ReportMerging #2920 will not alter performanceComparing
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
283-291: Prevent unnecessary Field import for ClassVar fields.The
importsproperty addsIMPORT_FIELDwhenself.fieldis truthy. For ClassVar fields,self.fieldreturns the default value representation (e.g.,'test') which is truthy, causing the Field import to be added even though ClassVar fields don't useField(...).🔎 Proposed fix
@property def imports(self) -> tuple[Import, ...]: """Get all required imports including Field if needed.""" # Fast path: skip expensive self.field check for simple required fields if self.required and not self.nullable and not self.alias and self.constraints is None and not self.extras: return super().imports - if self.field: + if self.field and not self.is_class_var: return chain_as_tuple(super().imports, (IMPORT_FIELD,)) return super().imports
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/model/pydantic/base_model.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-02T08:25:22.111Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2890
File: tests/data/expected/main/jsonschema/ref_nullable_with_constraint.py:14-15
Timestamp: 2026-01-02T08:25:22.111Z
Learning: The datamodel-code-generator currently generates RootModel subclasses with an explicit `root` field annotation (e.g., `class StringType(RootModel[str]): root: str`). This is existing behavior of the code generator and should not be flagged as an issue introduced by new changes.
Applied to files:
src/datamodel_code_generator/model/pydantic/base_model.py
📚 Learning: 2025-12-25T09:22:22.481Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2799
File: src/datamodel_code_generator/model/pydantic/__init__.py:43-43
Timestamp: 2025-12-25T09:22:22.481Z
Learning: In datamodel-code-generator project, defensive `# noqa: PLC0415` directives should be kept on lazy imports (imports inside functions/methods) even when Ruff reports them as unused via RUF100, to prepare for potential future Ruff configuration changes that might enable the import-outside-top-level rule.
Applied to files:
src/datamodel_code_generator/model/pydantic/base_model.py
📚 Learning: 2025-12-18T13:43:16.235Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2681
File: tests/cli_doc/test_cli_doc_coverage.py:82-82
Timestamp: 2025-12-18T13:43:16.235Z
Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.
Applied to files:
src/datamodel_code_generator/model/pydantic/base_model.py
🧬 Code graph analysis (1)
src/datamodel_code_generator/model/pydantic/base_model.py (2)
src/datamodel_code_generator/model/base.py (3)
repr_set_sorted(84-94)type_hint(278-296)annotated(422-424)src/datamodel_code_generator/model/msgspec.py (2)
type_hint(362-369)annotated(402-430)
🔇 Additional comments (5)
src/datamodel_code_generator/model/pydantic/base_model.py (5)
22-22: LGTM!The import of
repr_set_sortedis necessary for consistent default value representation in ClassVar fields (used on line 257).
256-258: LGTM!Correctly returns just the default value representation for ClassVar fields without wrapping in
Field(...), which aligns with how ClassVars are declared in Pydantic.
261-265: LGTM!The
is_class_varproperty correctly implements the JSON Schema custom propertyx-is-classvaras specified in issue #2919.
266-272: LGTM!The
type_hintproperty correctly wraps the parent's type hint withClassVar[...]when applicable. The commented-out debug code from the previous review has been properly removed.
278-279: LGTM!Correctly returns the ClassVar-wrapped type hint without the
Annotatedwrapper, which is appropriate since ClassVar fields don't useField(...)constraints.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
258-261: Consider moving ClassVar handling earlier in__str__for efficiency.The ClassVar early-return is placed after computing
field_argumentsand other processing. For ClassVar fields, most of this work (constraints, discriminator, default_factory logic) is unnecessary.🔎 Proposed optimization: move ClassVar check earlier
def __str__(self) -> str: # noqa: PLR0912 """Return Field() call with all constraints and metadata.""" + # Early return for ClassVar fields - they don't use Field() + if self.is_class_var: + if self.default is UNDEFINED: # pragma: no cover + return "" + return repr_set_sorted(self.default) if isinstance(self.default, set) else repr(self.default) + data: dict[str, Any] = {k: v for k, v in self.extras.items() if k not in self._EXCLUDE_FIELD_KEYS} # ... rest of method ... - - if self.is_class_var: - if self.default is UNDEFINED: # pragma: no cover - return "" - return repr_set_sorted(self.default) if isinstance(self.default, set) else repr(self.default) - return f"Field({', '.join(field_arguments)})"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/data/jsonschema/has_classvar_extra_annotated.jsonis excluded by!tests/data/**/*.jsonand included by nonetests/data/jsonschema/has_classvar_extra_set.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (4)
src/datamodel_code_generator/model/pydantic/base_model.pytests/data/expected/main/jsonschema/has_classvar_extra_annotated.pytests/data/expected/main/jsonschema/has_classvar_extra_set.pytests/main/jsonschema/test_main_jsonschema.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/main/jsonschema/test_main_jsonschema.py
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-02T08:25:22.111Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2890
File: tests/data/expected/main/jsonschema/ref_nullable_with_constraint.py:14-15
Timestamp: 2026-01-02T08:25:22.111Z
Learning: The datamodel-code-generator currently generates RootModel subclasses with an explicit `root` field annotation (e.g., `class StringType(RootModel[str]): root: str`). This is existing behavior of the code generator and should not be flagged as an issue introduced by new changes.
Applied to files:
tests/data/expected/main/jsonschema/has_classvar_extra_annotated.pytests/data/expected/main/jsonschema/has_classvar_extra_set.py
📚 Learning: 2025-12-25T09:22:22.481Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2799
File: src/datamodel_code_generator/model/pydantic/__init__.py:43-43
Timestamp: 2025-12-25T09:22:22.481Z
Learning: In datamodel-code-generator project, defensive `# noqa: PLC0415` directives should be kept on lazy imports (imports inside functions/methods) even when Ruff reports them as unused via RUF100, to prepare for potential future Ruff configuration changes that might enable the import-outside-top-level rule.
Applied to files:
src/datamodel_code_generator/model/pydantic/base_model.py
📚 Learning: 2025-12-18T13:43:16.235Z
Learnt from: koxudaxi
Repo: koxudaxi/datamodel-code-generator PR: 2681
File: tests/cli_doc/test_cli_doc_coverage.py:82-82
Timestamp: 2025-12-18T13:43:16.235Z
Learning: In datamodel-code-generator project, Ruff preview mode is enabled via `lint.preview = true` in pyproject.toml. This enables preview rules like PLR6301 (no-self-use), so `noqa: PLR6301` directives are necessary and should not be removed even if RUF100 suggests they are unused.
Applied to files:
src/datamodel_code_generator/model/pydantic/base_model.py
🧬 Code graph analysis (3)
tests/data/expected/main/jsonschema/has_classvar_extra_annotated.py (2)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
BaseModel(349-438)tests/data/expected/main/jsonschema/has_classvar_extra_set.py (1)
Model(12-13)
tests/data/expected/main/jsonschema/has_classvar_extra_set.py (2)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
BaseModel(349-438)tests/data/expected/main/jsonschema/has_classvar_extra_annotated.py (1)
Model(12-13)
src/datamodel_code_generator/model/pydantic/base_model.py (1)
src/datamodel_code_generator/model/base.py (1)
repr_set_sorted(84-94)
🔇 Additional comments (7)
tests/data/expected/main/jsonschema/has_classvar_extra_set.py (1)
1-13: LGTM!The test fixture correctly demonstrates ClassVar generation with a set type. The imports are properly included, and the
ClassVar[set[str] | None] = {'a', 'b'}syntax is valid for the expected Pydantic v2 model output.tests/data/expected/main/jsonschema/has_classvar_extra_annotated.py (1)
1-13: LGTM!The test fixture correctly demonstrates ClassVar generation with the
--use-annotatedoption. Notably, the ClassVar field intentionally does not use theAnnotated[]wrapper—this is correct behavior since ClassVar fields shouldn't have PydanticField()metadata.src/datamodel_code_generator/model/pydantic/base_model.py (5)
22-22: Acknowledge the cyclic import flagged by static analysis.The past review comments indicate a cyclic import issue at this line. While Python can often handle such cycles at runtime (especially when imports occur at module level before class definitions are evaluated), this is worth being aware of for future refactoring.
94-95: LGTM!Correctly returns
Nonefor ClassVar fields, preventingField()generation in the output. This aligns with Pydantic's behavior where ClassVars are not model fields.
265-268: LGTM!The property correctly checks for the
x-is-classvarextra key with an explicitis Truecomparison, ensuring only booleanTruetriggers ClassVar handling (not other truthy values).
270-275: LGTM!The
type_hintproperty correctly wraps the parent's type hint withClassVar[...]when applicable. The previously flagged debug code has been removed.
277-282: LGTM!Correctly skips
Annotated[]wrapping for ClassVar fields. ClassVars don't use PydanticField()metadata, so they shouldn't be wrapped inAnnotated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2920 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 92 92
Lines 16969 16989 +20
Branches 1976 1979 +3
=========================================
+ Hits 16969 16989 +20
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:
|
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR adds support for ClassVar in Pydantic v2 output via a new This analysis was performed by Claude Code Action |
|
@koxudaxi Thank you very much! |
|
🎉 Released in 0.53.0 This PR is now available in the latest release. See the release notes for details. |
3 participants
Fixes: #2919
While analysing a suitable approach, I was close to an implementation, so I already created the PR. I am happy to discuss the PR and make adjustments.
How should this "magic" behaviour be documented if this is a possible approach?
Thanks for your effort.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.