Support JSON files for mapping options#3071
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:
📝 WalkthroughWalkthroughCLI options Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI"
participant ArgParse as "Argument Parser\n(_json_value_or_file)"
participant FS as "Filesystem"
participant JSON as "JSON Parser"
participant Error as "Error Handler"
CLI->>ArgParse: receive --base-class-map / --enum-field-as-literal-map value
alt value is path to existing file
ArgParse->>FS: read file (DEFAULT_ENCODING)
FS-->>ArgParse: file contents / read error
alt read success
ArgParse->>JSON: json.loads(file contents)
JSON-->>ArgParse: parsed value / JSON error
else read error
ArgParse->>Error: raise ArgumentTypeError("Unable to read JSON file: ...")
end
else value is inline JSON
ArgParse->>JSON: json.loads(value)
JSON-->>ArgParse: parsed value / JSON error
end
alt parsed value is object (dict)
ArgParse-->>CLI: accept mapping dict
else not an object
ArgParse->>Error: raise ArgumentTypeError("Expected a JSON object")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
Generated by GitHub Actions
|
📚 Docs Preview: https://pr-3071.datamodel-code-generator.pages.dev |
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3071 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 87 87
Lines 18185 18237 +52
Branches 2085 2087 +2
=========================================
+ Hits 18185 18237 +52
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:
|
![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)
tests/main/jsonschema/test_main_jsonschema.py (1)
3015-3030: Consider adding failure-path parity for--base-class-mapfile input.Line 3015 adds happy-path coverage, but unlike
--enum-field-as-literal-map, there’s no invalid JSON / unreadable encoding test for--base-class-map. Adding those would keep both options equally protected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/jsonschema/test_main_jsonschema.py` around lines 3015 - 3030, Add failure-path tests for the --base-class-map option similar to the existing tests for --enum-field-as-literal-map: create two new tests (e.g., test_main_jsonschema_base_class_map_from_file_invalid_json and test_main_jsonschema_base_class_map_from_file_unreadable_encoding) that use run_main_and_assert (and the existing error/assert helper used by enum tests) to assert proper failure when mapping_path contains invalid JSON and when the file cannot be read with the expected encoding; reference the existing test_main_jsonschema_base_class_map_from_file, the --base-class-map flag, and helpers run_main_and_assert / assert_file_content to mirror the happy-path structure but assert error conditions.docs/cli-reference/typing-customization.md (1)
1538-1539: Consider adding a file-path example in the usage block.Since Line 1538 introduces file input support, adding a second usage example (e.g.,
--enum-field-as-literal-map ./enum-map.json) would make this easier to discover quickly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/cli-reference/typing-customization.md` around lines 1538 - 1539, Add a second usage example to the CLI usage block showing file-path input for the --enum-field-as-literal-map flag (e.g., `--enum-field-as-literal-map ./enum-map.json`) so readers can discover file-based mapping quickly; place this alongside the existing inline JSON example and mirror formatting/style used for other usage examples in the document.
🤖 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/arguments.py`:
- Around line 88-103: The _json_value_or_file function must validate that the
parsed JSON is a dict (mapping) because callers like the base-class-map and
enum-field-as-literal-map expect dict operations; after loading/parsing JSON,
check isinstance(result, dict) and if not raise ArgumentTypeError with a clear
message (e.g., "expected a JSON object/dict for …"); also update the function
return annotation from object to dict to reflect the correct type.
---
Nitpick comments:
In `@docs/cli-reference/typing-customization.md`:
- Around line 1538-1539: Add a second usage example to the CLI usage block
showing file-path input for the --enum-field-as-literal-map flag (e.g.,
`--enum-field-as-literal-map ./enum-map.json`) so readers can discover
file-based mapping quickly; place this alongside the existing inline JSON
example and mirror formatting/style used for other usage examples in the
document.
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 3015-3030: Add failure-path tests for the --base-class-map option
similar to the existing tests for --enum-field-as-literal-map: create two new
tests (e.g., test_main_jsonschema_base_class_map_from_file_invalid_json and
test_main_jsonschema_base_class_map_from_file_unreadable_encoding) that use
run_main_and_assert (and the existing error/assert helper used by enum tests) to
assert proper failure when mapping_path contains invalid JSON and when the file
cannot be read with the expected encoding; reference the existing
test_main_jsonschema_base_class_map_from_file, the --base-class-map flag, and
helpers run_main_and_assert / assert_file_content to mirror the happy-path
structure but assert error conditions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eaf9fe01-ddb6-4cef-9a51-c42ae7ed552b
⛔ Files ignored due to path filters (1)
docs/llms-full.txtis excluded by none and included by none
📒 Files selected for processing (4)
docs/cli-reference/model-customization.mddocs/cli-reference/typing-customization.mdsrc/datamodel_code_generator/arguments.pytests/main/jsonschema/test_main_jsonschema.py
Generated by GitHub Actions
🤖 Generated by GitHub Actions
Generated by GitHub Actions
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/main/jsonschema/test_main_jsonschema.py (1)
3032-3074: Consider parameterizing duplicated invalid-file-path tests.The invalid JSON / invalid encoding cases are duplicated for both options; a small parametrized helper would keep future message/behavior updates in one place.
Also applies to: 4670-4706
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/jsonschema/test_main_jsonschema.py` around lines 3032 - 3074, Replace the two nearly identical tests test_main_jsonschema_base_class_map_from_file_invalid_json and test_main_jsonschema_base_class_map_from_file_invalid_encoding with a single pytest.mark.parametrize test that iterates over tuples of (file_bytes_or_text, write_method, expected_error_substring) and writes the mapping file accordingly, then calls run_main_with_args(...) and asserts the SystemExit code and captured.err contains expected_error_substring; reuse the same parameters/approach for the duplicate set of tests noted elsewhere, and reference the existing helper run_main_with_args, JSON_SCHEMA_DATA_PATH, and the "--base-class-map" flag to locate the call sites to consolidate.
🤖 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/main/jsonschema/test_main_jsonschema.py`:
- Around line 4670-4706: Add a parity test that ensures the CLI rejects
non-object inline JSON passed to --enum-field-as-literal-map: create a new test
(e.g. test_main_enum_field_as_literal_map_non_object_inline_json) in
tests/main/jsonschema/test_main_jsonschema.py that calls run_main_with_args
using --enum-field-as-literal-map with an inline JSON value that is not an
object (for example "[]" or "1"), wrap the call with pytest.raises(SystemExit)
and assert exc_info.value.code == 2, then capture stderr and assert it contains
the expected error text indicating the JSON must be an object (mirroring the
existing base-class-map non-object inline JSON test logic but using the
--enum-field-as-literal-map option and the run_main_with_args helper).
---
Nitpick comments:
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 3032-3074: Replace the two nearly identical tests
test_main_jsonschema_base_class_map_from_file_invalid_json and
test_main_jsonschema_base_class_map_from_file_invalid_encoding with a single
pytest.mark.parametrize test that iterates over tuples of (file_bytes_or_text,
write_method, expected_error_substring) and writes the mapping file accordingly,
then calls run_main_with_args(...) and asserts the SystemExit code and
captured.err contains expected_error_substring; reuse the same
parameters/approach for the duplicate set of tests noted elsewhere, and
reference the existing helper run_main_with_args, JSON_SCHEMA_DATA_PATH, and the
"--base-class-map" flag to locate the call sites to consolidate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd974f76-748c-4b2e-bafc-91a250460228
📒 Files selected for processing (1)
tests/main/jsonschema/test_main_jsonschema.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/main/jsonschema/test_main_jsonschema.py (1)
3032-3074: Consider parameterizing the duplicated error-path tests for both map flags.The failure-case structures are nearly identical; a small parameterized helper would reduce maintenance overhead.
Also applies to: 4670-4724
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/main/jsonschema/test_main_jsonschema.py` around lines 3032 - 3074, The two tests test_main_jsonschema_base_class_map_from_file_invalid_json and test_main_jsonschema_base_class_map_from_file_invalid_encoding duplicate the same failure-path logic for the --base-class-map flag; refactor by parameterizing over the map flags (e.g., "--base-class-map" and the other map flag used elsewhere) using pytest.mark.parametrize or extracting a small helper function that accepts the flag name, writes the invalid file (or bytes), calls run_main_with_args and asserts SystemExit and the expected stderr text; update or add a single parametric test that iterates the two flags and the two invalid cases (invalid JSON text and invalid encoding) to replace the duplicated tests referenced (also apply same change to the similar block at lines 4670-4724).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/main/jsonschema/test_main_jsonschema.py`:
- Around line 3032-3074: The two tests
test_main_jsonschema_base_class_map_from_file_invalid_json and
test_main_jsonschema_base_class_map_from_file_invalid_encoding duplicate the
same failure-path logic for the --base-class-map flag; refactor by
parameterizing over the map flags (e.g., "--base-class-map" and the other map
flag used elsewhere) using pytest.mark.parametrize or extracting a small helper
function that accepts the flag name, writes the invalid file (or bytes), calls
run_main_with_args and asserts SystemExit and the expected stderr text; update
or add a single parametric test that iterates the two flags and the two invalid
cases (invalid JSON text and invalid encoding) to replace the duplicated tests
referenced (also apply same change to the similar block at lines 4670-4724).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0226c23-cc06-405c-b68b-a27ac374d787
📒 Files selected for processing (1)
tests/main/jsonschema/test_main_jsonschema.py
Breaking Change AnalysisResult: Breaking changes detected Reasoning: This PR contains multiple breaking changes beyond its stated scope of "JSON files for mapping options": (1) Major code generation output changes where default_factory lambdas are replaced with validate_default=True pattern across many test expected files, (2) Jinja2 template structural changes requiring custom template updates, (3) Removed public Python API classes (WrappedDefault, ValidatedDefault) and class variables, (4) Changed error handling from raw httpx exceptions to new SchemaFetchError, and (5) New deprecation warning for remote $ref fetching without explicit opt-in. Content for Release NotesCode Generation Changes
Custom Template Update Required
API/CLI Changes
Error Handling Changes
This analysis was performed by Claude Code Action |
|
🎉 Released in 0.56.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
fixes: #3068
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes