Extract shared pydantic base module for v2/dataclass/msgspec#3022
Extract shared pydantic base module for v2/dataclass/msgspec#3022
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCentralizes Pydantic v1/v2 base logic into a new Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
koxudaxi
force-pushed
the
pr1-extract-pydantic-base
branch
from
cb935b2 to
bfdac92
Compare
Merging this PR will degrade performance by 16.43%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | test_perf_stripe_style_pydantic_v2 |
1.7 s | 2 s | -14.74% |
| ❌ | WallTime | test_perf_duplicate_names |
856.3 ms | 1,013.9 ms | -15.55% |
| ❌ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.7 s | 2 s | -14.54% |
| ❌ | WallTime | test_perf_large_models_pydantic_v2 |
3.1 s | 3.7 s | -15.91% |
| ❌ | WallTime | test_perf_deep_nested |
5.1 s | 5.8 s | -12.93% |
| ❌ | WallTime | test_perf_multiple_files_input |
3.2 s | 3.8 s | -15.7% |
| ❌ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.2 s | 2.7 s | -16.16% |
| ❌ | WallTime | test_perf_graphql_style_pydantic_v2 |
712.5 ms | 834.4 ms | -14.62% |
| ❌ | WallTime | test_perf_complex_refs |
1.7 s | 2.1 s | -16.43% |
| ❌ | WallTime | test_perf_openapi_large |
2.5 s | 2.9 s | -13.69% |
| ❌ | WallTime | test_perf_all_options_enabled |
5.9 s | 6.8 s | -13.39% |
Comparing pr1-extract-pydantic-base (bfdac92) with main (66bc811)
Footnotes
-
98 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/datamodel_code_generator/model/pydantic_v2/types.py (1)
457-484: Rebuildtype_mapafter replacingself.data_typewithPydanticV2ContextDataType.
super().__init__()initializestype_map/strict_type_mapbefore Line 475 swapsself.data_type, so maps may stay bound to the pre-swap type class.♻️ Proposed adjustment
super().__init__( python_version=python_version, use_standard_collections=use_standard_collections, use_generic_container_types=use_generic_container_types, strict_types=strict_types, use_non_positive_negative_number_constrained_types=use_non_positive_negative_number_constrained_types, use_decimal_for_multiple_of=use_decimal_for_multiple_of, use_union_operator=use_union_operator, use_pendulum=use_pendulum, target_datetime_class=target_datetime_class, target_date_class=target_date_class, treat_dot_as_module=treat_dot_as_module, use_serialize_as_any=use_serialize_as_any, ) # Override the data_type with our pydantic v2 version from pydantic import create_model # noqa: PLC0415 self.data_type: type[DataType] = create_model( "PydanticV2ContextDataType", python_version=(PythonVersion, python_version), use_standard_collections=(bool, use_standard_collections), use_generic_container=(bool, use_generic_container_types), use_union_operator=(bool, use_union_operator), treat_dot_as_module=(bool, treat_dot_as_module), use_serialize_as_any=(bool, use_serialize_as_any), __base__=PydanticV2DataType, ) + self.type_map = self.type_map_factory( + self.data_type, + strict_types=self.strict_types, + pattern_key=self.PATTERN_KEY, + target_datetime_class=target_datetime_class, + target_date_class=target_date_class, + ) + self.strict_type_map = strict_type_map_factory(self.data_type)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/model/pydantic_v2/types.py` around lines 457 - 484, super().__init__() builds type_map/strict_type_map using the original data_type, but the code then replaces self.data_type with the PydanticV2ContextDataType model; to fix, after assigning self.data_type = create_model(..., __base__=PydanticV2DataType) ensure you rebuild/recompute the instance's type_map and strict_type_map so they bind to the new PydanticV2ContextDataType (e.g., call whatever internal helper or re-run the logic used by super().__init__ that populates type_map/strict_type_map, immediately after the self.data_type replacement).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/datamodel_code_generator/model/pydantic_v2/types.py`:
- Around line 457-484: super().__init__() builds type_map/strict_type_map using
the original data_type, but the code then replaces self.data_type with the
PydanticV2ContextDataType model; to fix, after assigning self.data_type =
create_model(..., __base__=PydanticV2DataType) ensure you rebuild/recompute the
instance's type_map and strict_type_map so they bind to the new
PydanticV2ContextDataType (e.g., call whatever internal helper or re-run the
logic used by super().__init__ that populates type_map/strict_type_map,
immediately after the self.data_type replacement).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4db51102-a0ca-4dec-be3e-95bf88fa78bc
📒 Files selected for processing (9)
src/datamodel_code_generator/model/dataclass.pysrc/datamodel_code_generator/model/msgspec.pysrc/datamodel_code_generator/model/pydantic/base_model.pysrc/datamodel_code_generator/model/pydantic/types.pysrc/datamodel_code_generator/model/pydantic_base.pysrc/datamodel_code_generator/model/pydantic_v2/base_model.pysrc/datamodel_code_generator/model/pydantic_v2/imports.pysrc/datamodel_code_generator/model/pydantic_v2/types.pysrc/datamodel_code_generator/parser/base.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3022 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 94 95 +1
Lines 18348 18389 +41
Branches 2129 2129
=========================================
+ Hits 18348 18389 +41
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 is a pure internal refactoring that consolidates duplicated code into a shared This analysis was performed by Claude Code Action |
|
🎉 Released in 0.55.0 This PR is now available in the latest release. See the release notes for details. |
2 participants
Summary by CodeRabbit