Fix all-export generation for hyphenated directories#3033
Fix all-export generation for hyphenated directories#3033koxudaxi merged 2 commits intokoxudaxi:mainfrom
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 (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughNormalizes module identifiers by replacing hyphens with underscores in the parser's export-collection logic; updates tests and expected fixtures to cover hyphenated directory names and package re-exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/datamodel_code_generator/parser/base.py (1)
2595-2604: Keep the self-module check in normalized space.Line 2601 compares
normalized_proc_moduleagainst the rawmoduletuple. It still works here because the laterdepth < 1check filters the self-module case, but hyphenated init packages end up relying on that side effect. Comparing againstnormalized_modulekeeps this helper consistent end-to-end.♻️ Small cleanup
- if not proc_models or normalized_proc_module == module: + if not proc_models or normalized_proc_module == normalized_module: continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/datamodel_code_generator/parser/base.py` around lines 2595 - 2604, The loop in the parser compares normalized_proc_module to the raw module tuple (`if not proc_models or normalized_proc_module == module:`) which mixes normalized and raw forms; change that comparison to use normalized_module (i.e., `normalized_proc_module == normalized_module`) so the self-module check is performed in normalized space alongside the earlier normalization steps for `module`, ensuring consistent behavior for hyphenated package names; update the condition in the for-loop that uses normalized_proc_module, module, and proc_models to reference normalized_module instead of module.
🤖 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/parser/base.py`:
- Around line 2595-2604: The loop in the parser compares normalized_proc_module
to the raw module tuple (`if not proc_models or normalized_proc_module ==
module:`) which mixes normalized and raw forms; change that comparison to use
normalized_module (i.e., `normalized_proc_module == normalized_module`) so the
self-module check is performed in normalized space alongside the earlier
normalization steps for `module`, ensuring consistent behavior for hyphenated
package names; update the condition in the for-loop that uses
normalized_proc_module, module, and proc_models to reference normalized_module
instead of module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05d0f8a3-e09d-47e8-9826-995921b48df9
⛔ Files ignored due to path filters (1)
tests/data/jsonschema/all_exports_hyphenated_directory/hyphenated-directory/order.jsonis excluded by!tests/data/**/*.jsonand included by none
📒 Files selected for processing (4)
src/datamodel_code_generator/parser/base.pytests/data/expected/main/jsonschema/all_exports_hyphenated_directory/hyphenated_directory/__init__.pytests/data/expected/main/jsonschema/all_exports_hyphenated_directory/hyphenated_directory/order.pytests/main/test_main_general.py
Fixes a bug where --all-exports-scope fails to generate __all__ imports for packages originating from schema directories with hyphens in the path.
868c3eb to
8d33a4a
Compare
Merging this PR will improve performance by 14.02%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | WallTime | test_perf_kubernetes_style_pydantic_v2 |
2.6 s | 2.4 s | +11.36% |
| ⚡ | WallTime | test_perf_stripe_style_pydantic_v2 |
2 s | 1.8 s | +10.59% |
| ⚡ | WallTime | test_perf_openapi_large |
3.1 s | 2.7 s | +14.01% |
| ⚡ | WallTime | test_perf_deep_nested |
6.2 s | 5.5 s | +12.28% |
| ⚡ | WallTime | test_perf_all_options_enabled |
6.9 s | 6.1 s | +12.99% |
| ⚡ | WallTime | test_perf_aws_style_openapi_pydantic_v2 |
1.9 s | 1.7 s | +11.57% |
| ⚡ | WallTime | test_perf_graphql_style_pydantic_v2 |
822.6 ms | 734.9 ms | +11.92% |
| ⚡ | WallTime | test_perf_duplicate_names |
1,069.4 ms | 937.9 ms | +14.02% |
| ⚡ | WallTime | test_perf_complex_refs |
2.2 s | 1.9 s | +12.51% |
| ⚡ | WallTime | test_perf_large_models_pydantic_v2 |
3.7 s | 3.3 s | +12.71% |
| ⚡ | WallTime | test_perf_multiple_files_input |
3.7 s | 3.3 s | +13.22% |
Comparing an5t:fix-all-exports-hyphenated (04e7236) with main (3c244da)
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. ↩
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3033 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 85 85
Lines 17684 17688 +4
Branches 2068 2068
=========================================
+ Hits 17684 17688 +4
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 bug fix. It corrects the handling of hyphenated directory names when using --all-exports-scope by normalizing paths (replacing hyphens with underscores) before comparing them. Previously, the feature was silently failing to generate all imports for packages in hyphenated directories. The fix makes the feature work correctly without changing any existing working behavior. No API/CLI changes, no template changes, no default behavior changes - just fixing broken functionality. 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. |
Fixes a bug where
--all-exports-scopefails to generate__all__imports for packages originating from schema directories with hyphens in the path.Summary by CodeRabbit
New Features
Bug Fixes
Tests