Add GenerateConfig for unified configuration#2829
Conversation
|
Warning Rate limit exceeded@koxudaxi has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds Pydantic-based configuration models (BaseConfig, GenerateConfig, ParserConfig, ParseConfig), auto-generated TypedDict modules under Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 (2)
src/datamodel_code_generator/__init__.py (1)
594-714: Config extraction block looks comprehensive but could benefit from a helper.The config extraction block correctly copies all fields from
GenerateConfigto local variables. However, this creates a maintenance burden since any new field added toGenerateConfigmust also be added here.Consider using a helper method or
**vars(config)pattern in the future to reduce duplication, though the current explicit approach has the advantage of being type-safe and self-documenting.src/datamodel_code_generator/_types/generate_config_dict.py (1)
34-44: Note: DataclassArguments is duplicated across generated files.The
DataclassArgumentsTypedDict appears in multiple generated_typesfiles (base_config_dict.py,parser_config_dict.py,generate_config_dict.py). This is expected behavior from the code generation process, as each file is self-contained. If reducing duplication becomes desirable, consider generating a shared types module.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/config-types.yamlpyproject.tomlsrc/datamodel_code_generator/__init__.pysrc/datamodel_code_generator/_types/__init__.pysrc/datamodel_code_generator/_types/base_config_dict.pysrc/datamodel_code_generator/_types/generate_config_dict.pysrc/datamodel_code_generator/_types/parse_config_dict.pysrc/datamodel_code_generator/_types/parser_config_dict.pysrc/datamodel_code_generator/config.pysrc/datamodel_code_generator/model/__init__.pysrc/datamodel_code_generator/model/types.pytests/main/test_main_general.pytests/model/test_base.pytox.ini
🧰 Additional context used
🧬 Code graph analysis (8)
tests/model/test_base.py (1)
src/datamodel_code_generator/model/types.py (2)
DataTypeManager(103-162)get_data_type(148-162)
src/datamodel_code_generator/model/types.py (2)
src/datamodel_code_generator/types.py (1)
Types(790-829)src/datamodel_code_generator/model/base.py (2)
path(907-909)name(826-828)
src/datamodel_code_generator/_types/parse_config_dict.py (3)
src/datamodel_code_generator/model/type_alias.py (1)
TypeAlias(37-42)src/datamodel_code_generator/enums.py (3)
AllExportsCollisionStrategy(92-102)AllExportsScope(81-89)ModuleSplitMode(172-178)src/datamodel_code_generator/config.py (1)
ParseConfig(212-225)
src/datamodel_code_generator/model/__init__.py (1)
src/datamodel_code_generator/format.py (1)
PythonVersion(68-140)
src/datamodel_code_generator/_types/generate_config_dict.py (6)
src/datamodel_code_generator/enums.py (5)
AllExportsCollisionStrategy(92-102)AllExportsScope(81-89)DataModelType(48-56)InputFileType(35-45)ModuleSplitMode(172-178)src/datamodel_code_generator/_types/base_config_dict.py (1)
DataclassArguments(26-36)src/datamodel_code_generator/_types/parser_config_dict.py (1)
DataclassArguments(26-36)src/datamodel_code_generator/format.py (2)
DateClassType(60-65)DatetimeClassType(50-57)src/datamodel_code_generator/parser/__init__.py (1)
LiteralType(20-25)src/datamodel_code_generator/config.py (1)
GenerateConfig(165-189)
src/datamodel_code_generator/config.py (2)
src/datamodel_code_generator/enums.py (3)
AllExportsCollisionStrategy(92-102)AllExportsScope(81-89)DataModelType(48-56)src/datamodel_code_generator/parser/__init__.py (1)
LiteralType(20-25)
src/datamodel_code_generator/_types/base_config_dict.py (4)
src/datamodel_code_generator/enums.py (3)
AllOfMergeMode(142-152)CollapseRootModelsNameStrategy(131-139)DataModelType(48-56)src/datamodel_code_generator/_types/generate_config_dict.py (1)
DataclassArguments(34-44)src/datamodel_code_generator/_types/parser_config_dict.py (1)
DataclassArguments(26-36)src/datamodel_code_generator/format.py (3)
DateClassType(60-65)DatetimeClassType(50-57)PythonVersion(68-140)
src/datamodel_code_generator/__init__.py (2)
src/datamodel_code_generator/_types/generate_config_dict.py (1)
GenerateConfig(100-221)src/datamodel_code_generator/config.py (1)
GenerateConfig(165-189)
🪛 Ruff (0.14.10)
src/datamodel_code_generator/__init__.py
1206-1206: Unused noqa directive (non-enabled: PLC0415)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (23)
src/datamodel_code_generator/model/types.py (2)
65-65: LGTM! Type mappings are consistent.The additions of
Types.ulidandTypes.pathmappings todata_type_strfollow the established pattern. Both default tostr, withTypes.pathbeing overridden topathlib.Pathwhenuse_standard_primitive_types=True(line 99).Also applies to: 72-72
156-162: LGTM! Improved error handling with helpful message.The explicit NotImplementedError provides a clear, actionable error message that directs users to report missing type mappings. This is much better than an implicit KeyError and is covered by the new test in
tests/model/test_base.py.tests/main/test_main_general.py (3)
873-895: LGTM! Good coverage of the new GenerateConfig API.The test demonstrates the config parameter usage and validates that options are correctly extracted and applied. The snapshot assertion ensures the output format remains consistent.
898-904: LGTM! Validates lazy import mechanism.The test confirms that
GenerateConfigcan be accessed via the lazy import path, which is important for backward compatibility and convenience.
907-912: LGTM! Proper error handling test.The test ensures that the
__getattr__implementation raises the appropriateAttributeErrorfor non-existent attributes, preventing confusing errors downstream.src/datamodel_code_generator/_types/__init__.py (1)
1-11: LGTM! Clear documentation of auto-generated types.The docstring effectively explains the purpose of the module, how the TypedDict files are generated, and includes an appropriate warning against manual edits.
tox.ini (1)
115-123: LGTM! Well-defined test environment for config type generation.The
config-typesenvironment correctly runs all four profiling commands to generate TypedDict representations from the config models. This dogfooding approach is elegant and ensures type consistency.tests/model/test_base.py (1)
431-439: LGTM! Good test coverage for error handling.The test validates the new
NotImplementedErrorbehavior inget_data_typeby removing a type mapping and asserting the expected error message. This ensures robust error reporting for unmapped types..github/workflows/config-types.yaml (1)
1-70: LGTM! Well-designed automation with appropriate security.The workflow appropriately automates TypedDict generation when
config.pychanges. The use ofpull_request_targetis properly restricted to trusted actors and requires explicit labeling, which provides good security.pyproject.toml (1)
261-287: LGTM! Consistent profiling sections for config type generation.The four profiling sections are well-structured and consistent. The
type-overridesforpathensures thatpathlib.Pathis preserved in the TypedDict representation, anddisable-timestampensures reproducible, version-control-friendly output.src/datamodel_code_generator/config.py (4)
52-162: LGTM! Well-structured base configuration.The
BaseConfigclass consolidates 88 common parameters with appropriate type annotations and sensible defaults. The use ofConfigDict(extra="forbid")provides strict validation that will catch configuration typos.
165-189: LGTM! Clean extension with generate()-specific parameters.The
GenerateConfigclass appropriately extendsBaseConfigwith parameters specific to thegenerate()function, including input/output handling and HTTP settings.
192-209: LGTM! Parser-specific configuration is well-defined.The
ParserConfigclass extendsBaseConfigwith parameters needed for Parser initialization. The duplication of HTTP parameters withGenerateConfigis justified since both contexts need HTTP settings independently.
212-225: LGTM! Focused configuration for parse() method.The
ParseConfigclass appropriately extendsBaseModeldirectly (notBaseConfig) since these are parse-time settings rather than code generation settings. This is a good separation of concerns.src/datamodel_code_generator/__init__.py (3)
1203-1209: LGTM! Lazy import pattern is correctly implemented.The
__getattr__implementation properly handles lazy loading ofGenerateConfigto avoid circular imports. The error handling for unknown attributes follows Python conventions.Per static analysis: the
noqa: PLC0415directive on line 1206 appears to be unnecessary since function-level imports are intentional here. You can safely remove it.
60-66: LGTM! TYPE_CHECKING import correctly set up.The
TYPE_CHECKINGimport ofGenerateConfigenables static type checkers to resolve the type without triggering runtime circular imports. Combined with the lazy__getattr__, this is a clean solution.
451-455: LGTM! Config parameter maintains backward compatibility.The optional
configparameter allows users to migrate to the structured configuration approach at their own pace. Existing code using individual parameters continues to work unchanged.src/datamodel_code_generator/_types/parse_config_dict.py (1)
1-26: LGTM! Generated TypedDict correctly mirrors the ParseConfig model.This auto-generated file provides TypedDict representation for
ParseConfig, with all fields correctly marked asNotRequiredto match the Pydantic model's optional fields. TheLiteraltype aliases align with the corresponding Enum values defined inenums.py.src/datamodel_code_generator/_types/generate_config_dict.py (2)
1-98: Generated TypeAliases correctly represent enum values.The TypeAliases accurately reflect the string values of their corresponding Enums. For example,
DataModelTypeincludes all six model types matchingenums.DataModelType, andPythonVersioncovers '3.10' through '3.14' matchingformat.PythonVersion.
100-221: LGTM! GenerateConfig TypedDict is comprehensive.The
GenerateConfigTypedDict covers all configuration options from thegenerate()function, with correct types andNotRequiredannotations matching the optional nature of these parameters.src/datamodel_code_generator/_types/parser_config_dict.py (1)
84-198: LGTM! ParserConfig TypedDict includes parser-specific fields.The
ParserConfigTypedDict correctly includes parser-specific fields likebase_path,allow_responses_without_content,known_third_party,defer_formatting,target_datetime_class,target_date_class, anddefault_field_extrasthat are used during the parsing phase but not exposed in the publicGenerateConfig.src/datamodel_code_generator/_types/base_config_dict.py (2)
84-187: LGTM! BaseConfig TypedDict provides the shared configuration surface.The
BaseConfigTypedDict correctly captures the shared configuration options that form the basis forGenerateConfigand other config types. All fields are appropriately marked asNotRequiredwith types matching the Pydantic model definitions inconfig.py.
10-81: LGTM! TypeAliases correctly mirror enum values.All TypeAliases use
Literaltypes that accurately represent the string values of their corresponding Enums:
AllOfMergeModematchesenums.AllOfMergeModevaluesDataModelTypematchesenums.DataModelTypevaluesPythonVersionmatchesformat.PythonVersionmembers
| from typing import TYPE_CHECKING, NamedTuple | ||
|
|
||
| from datamodel_code_generator import PythonVersion | ||
| from datamodel_code_generator.format import PythonVersion |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all imports of PythonVersion to verify consistency
echo "=== Old-style imports (from datamodel_code_generator import PythonVersion) ==="
rg -n "from datamodel_code_generator import.*PythonVersion" --type=py
echo ""
echo "=== New-style imports (from datamodel_code_generator.format import PythonVersion) ==="
rg -n "from datamodel_code_generator\.format import.*PythonVersion" --type=py
echo ""
echo "=== All imports containing PythonVersion ==="
rg -n "import.*PythonVersion|PythonVersion.*import" --type=pyRepository: koxudaxi/datamodel-code-generator
Length of output: 3060
🏁 Script executed:
#!/bin/bash
# Verify PythonVersion is defined in format module
echo "=== Searching for PythonVersion definition ==="
rg -n "^class PythonVersion|^PythonVersion\s*=" src/datamodel_code_generator/format.py --type=py -A 5
echo ""
echo "=== Check if PythonVersion is exported from main __init__.py ==="
rg -n "PythonVersion" src/datamodel_code_generator/__init__.py --type=pyRepository: koxudaxi/datamodel-code-generator
Length of output: 497
Update import path in msgspec.py and types.py for consistency.
The change to use from datamodel_code_generator.format import PythonVersion in the reviewed file is correct. However, the refactoring is incomplete: src/datamodel_code_generator/model/msgspec.py (line 13) and src/datamodel_code_generator/model/types.py (line 10) still import PythonVersion from the root datamodel_code_generator module.
While the old import path still works due to re-export in __init__.py, update these files to use the new import path for consistency:
src/datamodel_code_generator/model/msgspec.py:13src/datamodel_code_generator/model/types.py:10
🤖 Prompt for AI Agents
In src/datamodel_code_generator/model/msgspec.py around line 13 and
src/datamodel_code_generator/model/types.py around line 10, update the
PythonVersion import to the new module path for consistency: replace imports
that reference PythonVersion from the package root with "from
datamodel_code_generator.format import PythonVersion", remove the old root-level
import, and ensure any usages continue to reference the same symbol.
🤖 Generated by GitHub Actions
|
📚 Docs Preview: https://pr-2829.datamodel-code-generator.pages.dev |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def test_generate_config_lazy_import() -> None: | ||
| """Test that GenerateConfig can be imported via __getattr__ lazy import.""" | ||
| import datamodel_code_generator |
Check notice
Code scanning / CodeQL
Module is imported with 'import' and 'import from' Note test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix “module imported with both import and from ... import ...”, keep one import style and avoid re-importing the same module with a different style. If you need both specific attributes and the module object, prefer a single import module and reference attributes as module.attr, or keep just the from ... import ... and refer to the module via an alias or variable obtained from that import.
For this file, the simplest fix without changing functionality is to remove the late import datamodel_code_generator statements inside the two tests and instead bind the already-imported module to a local variable using sys.modules["datamodel_code_generator"]. This avoids adding new top-level imports or changing existing ones, and preserves the behavior that these tests are working with the runtime module object (including its __getattr__ behavior and any lazy imports). Specifically:
- In
test_generate_config_lazy_import, replaceimport datamodel_code_generatorwithdatamodel_code_generator = sys.modules["datamodel_code_generator"]. - In
test_module_getattr_raises_for_unknown_attribute, do the same replacement. - To support this, add a single
import sysnear the top of the file alongside the other imports.
No other changes to tests or behavior are needed.
| @@ -8,6 +8,7 @@ | ||
| import black | ||
| import pytest | ||
| from inline_snapshot import snapshot | ||
| import sys | ||
|
|
||
| from datamodel_code_generator import ( | ||
| AllExportsScope, | ||
| @@ -1897,7 +1898,7 @@ | ||
|
|
||
| def test_generate_config_lazy_import() -> None: | ||
| """Test that GenerateConfig can be imported via __getattr__ lazy import.""" | ||
| import datamodel_code_generator | ||
| datamodel_code_generator = sys.modules["datamodel_code_generator"] | ||
|
|
||
| config_class = datamodel_code_generator.GenerateConfig | ||
| assert config_class is not None | ||
| @@ -1906,7 +1907,7 @@ | ||
|
|
||
| def test_module_getattr_raises_for_unknown_attribute() -> None: | ||
| """Test that __getattr__ raises AttributeError for unknown attributes.""" | ||
| import datamodel_code_generator | ||
| datamodel_code_generator = sys.modules["datamodel_code_generator"] | ||
|
|
||
| with pytest.raises(AttributeError, match="module 'datamodel_code_generator' has no attribute 'NonExistentClass'"): | ||
| _ = datamodel_code_generator.NonExistentClass |
|
|
||
| def test_module_getattr_raises_for_unknown_attribute() -> None: | ||
| """Test that __getattr__ raises AttributeError for unknown attributes.""" | ||
| import datamodel_code_generator |
Check notice
Code scanning / CodeQL
Module is imported with 'import' and 'import from' Note test
Copilot Autofix
AI 4 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
CodSpeed Performance ReportMerging #2829 will not alter performanceComparing
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/datamodel_code_generator/config.py (3)
23-24: Remove unused noqa directives.The
noqadirectives forD101andD105are unnecessary since these docstring rules are not enabled in the project's Ruff configuration.🔎 Proposed fix
- class SkipJsonSchema: # noqa: D101 - def __class_getitem__(cls, item: Any) -> Any: # noqa: D105 + class SkipJsonSchema: + def __class_getitem__(cls, item: Any) -> Any: return itemBased on static analysis hints.
215-218: Consider extracting duplicated HTTP configuration.The HTTP-related parameters (
http_headers,http_ignore_tls,http_timeout,http_query_parameters) are duplicated betweenGenerateConfig(lines 190-193) andParserConfig(lines 215-218).While this duplication may be intentional for API flexibility, it creates maintenance overhead if types or defaults need to change. Consider extracting these into a separate mixin class or nested config object.
💡 Example approach using a mixin
class HttpConfigMixin(BaseModel): """Mixin for HTTP-related configuration.""" http_headers: Sequence[tuple[str, str]] | None = None http_ignore_tls: bool = False http_timeout: float | None = None http_query_parameters: Sequence[tuple[str, str]] | None = None class GenerateConfig(BaseConfig, HttpConfigMixin): # ... other fields class ParserConfig(BaseConfig, HttpConfigMixin): # ... other fields
230-234: Consider extracting duplicated parse-phase configuration.The parse-phase parameters (
settings_path,disable_future_imports,all_exports_scope,all_exports_collision_strategy,module_split_mode) are duplicated betweenGenerateConfig(lines 194-198) andParseConfig(lines 230-234).Similar to the HTTP configuration duplication, this creates maintenance overhead. Consider extracting these into a shared structure.
💡 Example approach using composition
class ParsePhaseConfig(BaseModel): """Configuration for the parsing phase.""" settings_path: Path | None = None disable_future_imports: bool = False all_exports_scope: AllExportsScope | None = None all_exports_collision_strategy: AllExportsCollisionStrategy | None = None module_split_mode: ModuleSplitMode | None = None class GenerateConfig(BaseConfig): # ... other fields parse_phase: ParsePhaseConfig | None = None class ParseConfig(BaseModel): model_config = ConfigDict(extra="forbid") parse_phase: ParsePhaseConfig
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/datamodel_code_generator/config.py
🧰 Additional context used
🪛 Ruff (0.14.10)
src/datamodel_code_generator/config.py
23-23: Unused noqa directive (non-enabled: D101)
Remove unused noqa directive
(RUF100)
24-24: Unused noqa directive (non-enabled: D105)
Remove unused noqa directive
(RUF100)
⏰ 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). (11)
- GitHub Check: 3.10 on Ubuntu
- GitHub Check: py312-black23 on Ubuntu
- GitHub Check: 3.12 on Windows
- GitHub Check: 3.14 on Windows
- GitHub Check: 3.10 on Windows
- GitHub Check: 3.14 on macOS
- GitHub Check: 3.11 on macOS
- GitHub Check: 3.11 on Windows
- GitHub Check: 3.13 on Windows
- GitHub Check: Analyze (python)
- GitHub Check: benchmarks
🔇 Additional comments (1)
src/datamodel_code_generator/config.py (1)
61-172: LGTM! Well-structured base configuration.The BaseConfig class is well-designed with:
- Strict validation via
extra="forbid"to catch typos- Comprehensive type hints for all 88+ parameters
- Sensible defaults making all fields optional
- Proper use of
SkipJsonSchemato exclude the callable field from JSON schema generation
🤖 Generated by GitHub Actions
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2829 +/- ##
==========================================
- Coverage 99.52% 96.51% -3.01%
==========================================
Files 89 94 +5
Lines 13963 14721 +758
Branches 1665 1669 +4
==========================================
+ Hits 13896 14208 +312
- Misses 36 481 +445
- Partials 31 32 +1
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:
|
koxudaxi
force-pushed
the
feature/generate-config
branch
from
8d7905d to
e0726ee
Compare
🤖 Generated by GitHub Actions
- Add config: GenerateConfig | None parameter to generate() - Add exclusivity check: config and **options cannot be used together - Create GenerateConfig from options when options are provided - Convert extra_template_data dict to defaultdict for compatibility - Update UnionModeType to accept UnionMode enum - Add test for config and options exclusivity check 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated by GitHub Actions
2 participants
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.