Add __hash__ to Pydantic v2 models used in sets#2918
Conversation
📝 WalkthroughWalkthroughAdds a per-model Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Generator/CLI
participant Parser as Parser
participant Registry as Model Registry
participant Renderer as Template Renderer
participant FS as File Output
CLI->>Parser: parse schemas (unique-items-as-set enabled)
Parser->>Registry: build DataModel instances
note right of Registry `#E8F5E9`: DataModel.extra_template_data\ninitialized per-instance
Parser->>Parser: collect set/frozenset item type references
Parser->>Registry: mark referenced models (set_item_hashable = True)
Registry->>Renderer: render models (reads extra_template_data / flags)
Renderer->>FS: write files (emit __hash__ where flagged)
FS-->>CLI: generation complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (12)
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-2918.datamodel-code-generator.pages.dev |
CodSpeed Performance ReportMerging #2918 will not alter performanceComparing
|
![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
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/base.py (1)
681-693: Type annotation mismatch with defaultdict usage.Line 681 declares
extra_template_data: dict[str, Any], but line 693 assignsdefaultdict(dict)when the parameter is None. While defaultdict is a subclass of dict, the type annotation should accurately reflect the actual type for proper type checking.🔎 Proposed fix
- self.extra_template_data: dict[str, Any] + self.extra_template_data: dict[str, Any] | defaultdict[str, dict[str, Any]] if extra_template_data is not None:Alternatively, ensure consistent typing by always using dict:
else: - self.extra_template_data = defaultdict(dict) + self.extra_template_data = {}
🧹 Nitpick comments (1)
tests/main/jsonschema/test_main_jsonschema.py (1)
7908-7923: Enum/set regression test is correctly wired; minor docstring nit onlyThe test is consistent with the rest of the suite: correct skip marker for Pydantic v2, expected fixture path, and CLI args that exercise
--use-unique-items-as-setwith standard collections. This should reliably catch regressions in the codegen for enum-in-set scenarios.If you want to be slightly clearer, you could extend the docstring to also mention the positive case (that the generated item model in the same fixture gets
__hash__) since the golden file likely asserts both aspects, not just “no__hash__on the enum”. Otherwise this looks good as-is.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
src/datamodel_code_generator/model/template/pydantic_v2/BaseModel.jinja2is excluded by none and included by nonesrc/datamodel_code_generator/model/template/pydantic_v2/RootModel.jinja2is excluded by none and included by nonetests/data/jsonschema/unique_items_enum_set.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (6)
src/datamodel_code_generator/model/base.pysrc/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/unique_items_enum_set.pytests/data/expected/main/openapi/with_field_constraints_pydantic_v2_use_generic_container_types_set.pytests/data/expected/main/openapi/with_field_constraints_pydantic_v2_use_standard_collections_set.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:
src/datamodel_code_generator/model/base.py
🧬 Code graph analysis (2)
tests/data/expected/main/jsonschema/unique_items_enum_set.py (2)
src/datamodel_code_generator/model/enum.py (1)
Enum(39-121)src/datamodel_code_generator/model/base.py (1)
name(829-831)
src/datamodel_code_generator/parser/base.py (2)
src/datamodel_code_generator/model/base.py (2)
all_data_types(877-881)path(910-912)src/datamodel_code_generator/types.py (1)
all_data_types(576-582)
🪛 GitHub Actions: Lint
src/datamodel_code_generator/parser/base.py
[error] 1551-1552: PLR6301 Method __mark_set_item_models_hashable could be a function, class method, or static method
[error] 1555-1563: PLR1702 Too many nested blocks (7 > 5)
⏰ 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). (12)
- GitHub Check: 3.10 on Windows
- GitHub Check: py312-pydantic1 on Ubuntu
- GitHub Check: py312-isort5 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: 3.12 on macOS
- GitHub Check: 3.11 on Ubuntu
- GitHub Check: 3.12 on Ubuntu
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: benchmarks
- GitHub Check: Analyze (python)
🔇 Additional comments (4)
tests/data/expected/main/openapi/with_field_constraints_pydantic_v2_use_standard_collections_set.py (1)
10-15: LGTM! Correct implementation of hashability for set usage.The addition of
__hash__ = object.__hash__correctly enables Pet instances to be used in sets. This matches the solution described in issue #1614 and follows Pydantic v2 best practices for making model instances hashable.tests/data/expected/main/jsonschema/unique_items_enum_set.py (1)
1-25: LGTM! Demonstrates correct discriminated hashability handling.This test fixture correctly shows that:
- Enum types (Status) don't receive
__hash__since they're already hashable by default- Pydantic models (Item) used in sets receive
__hash__ = object.__hash__This validates that the
__mark_set_item_models_hashablemethod properly skips Enum types while marking BaseModel types.tests/data/expected/main/openapi/with_field_constraints_pydantic_v2_use_generic_container_types_set.py (1)
12-16: LGTM! Validates frozenset support.The addition of
__hash__ = object.__hash__correctly enables Pet instances to be used in frozensets. This confirms that the implementation handles bothsetandfrozensetcollection types appropriately.src/datamodel_code_generator/parser/base.py (1)
2953-2954: LGTM! Correct integration in processing flow.The call to
__mark_set_item_models_hashableis correctly placed immediately after__replace_unique_list_to_set, ensuring that models are marked as hashable after their field types have been converted from list to set.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2918 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 93 93
Lines 16865 16913 +48
Branches 1952 1966 +14
=========================================
+ Hits 16865 16913 +48
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 This analysis was performed by Claude Code Action |
|
🎉 Released in 0.52.2 This PR is now available in the latest release. See the release notes for details. |
1 participant
Fixes: #1614
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.