Keep generate string input inline#3573
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 ignored due to path filters (1)
📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR changes ChangesPath string input handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant generate
participant Parser
Caller->>generate: generate(input_=string)
generate->>generate: infer / normalize / build parser
alt failure before parse
generate->>generate: warn if string resolves to existing path
generate-->>Caller: re-raise original error
else parser.parse() fails
generate->>Parser: parser.parse(...)
Parser-->>generate: raise error
generate->>Parser: parser._dispose()
generate-->>Caller: re-raise original error
end
Possibly related PRs
Suggested labels: bug, tests, documentation Suggested reviewers: koxudaxi 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
📚 Docs Preview: https://pr-3573.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 #3573 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 163 163
Lines 34977 35010 +33
Branches 4047 4047
=========================================
+ Hits 34977 35010 +33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 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: 2
🧹 Nitpick comments (1)
src/datamodel_code_generator/__init__.py (1)
1456-1456: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated warn+re-raise boilerplate across 5 call sites.
The
except Exception: _warn_if_input_string_points_to_existing_path(input_); raisepattern is duplicated near-verbatim at each failure point (type inference, normalization, MCP conversion, parser construction, parse). Consider a small context manager to consolidate this and reduce drift risk if the pattern needs to change later.♻️ Example consolidation
+@contextlib.contextmanager +def _warn_on_string_path_failure(input_: Path | str | ParseResult | Mapping[str, Any] | list[Any]) -> Iterator[None]: + try: + yield + except Exception: + _warn_if_input_string_points_to_existing_path(input_) + raiseThen wrap each block, e.g.:
- try: - input_text = _normalize_raw_input(input_, input_text, input_file_type, config) - except Exception: - _warn_if_input_string_points_to_existing_path(input_) - raise + with _warn_on_string_path_failure(input_): + input_text = _normalize_raw_input(input_, input_text, input_file_type, config)Also applies to: 1468-1484, 1505-1520, 1532-1536
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/datamodel_code_generator/__init__.py` at line 1456, The same warn-and-reraise pattern is duplicated across the input handling flow in __init__.py, so consolidate it into a small reusable helper or context manager. Update the failure paths around the type inference, normalization, MCP conversion, parser construction, and parse logic to use that shared wrapper instead of repeating try/except with _warn_if_input_string_points_to_existing_path(input_) and raise, keeping the existing warning behavior via the same function names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/using_as_module.md`:
- Around line 47-48: The documentation for input_ needs to reflect the full
string-handling behavior, not just that strings are schema text. Update the
wording in the module usage section to clarify that Path should be used for file
input, while string values are interpreted as schema text unless they point to
an existing path, in which case the code handles them specially and may warn or
recommend Path. Make sure the text around input_ accurately distinguishes Path,
schema text, and existing-path strings using the input_ parameter description as
the anchor.
In `@src/datamodel_code_generator/__init__.py`:
- Around line 762-784: The warning in
_warn_if_input_string_points_to_existing_path can be promoted to an exception
under strict warning filters and mask the original failure in generate(). Make
the warning emission safe in that helper by guarding it so it cannot replace an
active exception (for example, avoid raising from warnings.warn when called
during error handling), and ensure the existing InvalidFileFormatError/parser
exception remains the one propagated.
---
Nitpick comments:
In `@src/datamodel_code_generator/__init__.py`:
- Line 1456: The same warn-and-reraise pattern is duplicated across the input
handling flow in __init__.py, so consolidate it into a small reusable helper or
context manager. Update the failure paths around the type inference,
normalization, MCP conversion, parser construction, and parse logic to use that
shared wrapper instead of repeating try/except with
_warn_if_input_string_points_to_existing_path(input_) and raise, keeping the
existing warning behavior via the same function names.
🪄 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: 5f7c25a1-ea20-4027-aae4-7cc60764ed75
⛔ Files ignored due to path filters (1)
docs/llms-full.txtis excluded by none and included by none
📒 Files selected for processing (6)
docs/using_as_module.mdsrc/datamodel_code_generator/__init__.pytests/data/expected/main/generate_accepts_path_input.pytests/data/expected/main/generate_keeps_existing_path_string_input.pytests/data/expected/main/generate_keeps_non_path_string_input.pytests/main/test_main_general.py
Breaking Change AnalysisResult: No breaking changes detected Reasoning: This PR reworks how This analysis was performed by Claude Code Action |
|
🎉 Released in 0.68.0 This PR is now available in the latest release. See the release notes for details. |
1 participant
Fixes: #2381
Summary by CodeRabbit
Documentation
Bug Fixes
Tests
Pathvs string input behavior, including warning and failure cases.