Reorganize#356
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR reorganizes module structure across the codebase: private helper functions ( Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
schemas/datasets.DatasetStatusthe docstring has a small typo (issn't) which should be corrected for clarity. - Several helpers that are effectively part of the public API still have leading underscores and are imported across modules (e.g.
_format_parquet_url); consider either making their names public (dropping the underscore) or keeping them internal to a single module for consistency with the other renamed helpers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `schemas/datasets.DatasetStatus` the docstring has a small typo (`issn't`) which should be corrected for clarity.
- Several helpers that are effectively part of the public API still have leading underscores and are imported across modules (e.g. `_format_parquet_url`); consider either making their names public (dropping the underscore) or keeping them internal to a single module for consistency with the other renamed helpers.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (99.25%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #356 +/- ##
==========================================
+ Coverage 94.51% 94.56% +0.05%
==========================================
Files 77 77
Lines 3792 3774 -18
Branches 248 247 -1
==========================================
- Hits 3584 3569 -15
+ Misses 140 137 -3
Partials 68 68 ☔ 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: 1
🧹 Nitpick comments (1)
src/schemas/core.py (1)
7-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicate
Visibilityenum.
Visibilityis defined in bothschemas/core.py(usingauto()) andschemas/datasets.py(using explicit"public"/"private"). Both produce equivalent values, butaccess.pyimports fromschemas.datasetswhilestudy.pyimports fromschemas.core. This PR's goal of consolidation makes the duplication worth resolving — pick one canonical location and re-export or import from the other.🤖 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/schemas/core.py` around lines 7 - 10, The `Visibility` enum is duplicated across `schemas/core.py` and `schemas/datasets.py`, so consolidate it into one canonical definition and make the other module re-export or import that same symbol. Update the `Visibility` class in the chosen home module and then fix `access.py` and `study.py` to import the shared enum consistently so both code paths use the same `Visibility` type.
🤖 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 `@src/schemas/datasets.py`:
- Line 29: The DatasetStatus docstring contains a typo in the “In Preparation”
description. Update the text in the DatasetStatus definition so the word
“issn't” is corrected to “isn't”, keeping the rest of the status description
unchanged.
---
Nitpick comments:
In `@src/schemas/core.py`:
- Around line 7-10: The `Visibility` enum is duplicated across `schemas/core.py`
and `schemas/datasets.py`, so consolidate it into one canonical definition and
make the other module re-export or import that same symbol. Update the
`Visibility` class in the chosen home module and then fix `access.py` and
`study.py` to import the shared enum consistently so both code paths use the
same `Visibility` type.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b5cc8057-fb67-4b8d-9177-7bd5244d2105
📒 Files selected for processing (67)
src/core/access.pysrc/core/conversions.pysrc/core/formatting.pysrc/core/types.pysrc/database/datasets.pysrc/database/engine.pysrc/database/evaluations.pysrc/database/flows.pysrc/database/models/__init__.pysrc/database/models/base.pysrc/database/models/setups.pysrc/database/models/tags.pysrc/database/qualities.pysrc/database/runs.pysrc/database/setups.pysrc/database/studies.pysrc/database/tasks.pysrc/database/users.pysrc/main.pysrc/routers/__init__.pysrc/routers/datasets.pysrc/routers/dependencies.pysrc/routers/estimation_procedure.pysrc/routers/evaluations.pysrc/routers/flows.pysrc/routers/openml/__init__.pysrc/routers/qualities.pysrc/routers/runs.pysrc/routers/setups.pysrc/routers/study.pysrc/routers/tasks.pysrc/routers/tasktype.pysrc/routers/users.pysrc/schemas/core.pysrc/schemas/datasets.pysrc/schemas/datasets/__init__.pysrc/schemas/tasks.pytests/conftest.pytests/routers/dataset_tag_test.pytests/routers/dataset_untag_test.pytests/routers/datasets_features_test.pytests/routers/datasets_get_test.pytests/routers/datasets_list_datasets_test.pytests/routers/datasets_qualities_test.pytests/routers/datasets_status_test.pytests/routers/estimation_procedure_test.pytests/routers/evaluation_measure_test.pytests/routers/flows_exists_test.pytests/routers/flows_get_test.pytests/routers/openml/__init__.pytests/routers/qualities_list_test.pytests/routers/runs_get_test.pytests/routers/runs_trace_test.pytests/routers/setups_get_test.pytests/routers/setups_tag_test.pytests/routers/setups_untag_test.pytests/routers/study_attach_test.pytests/routers/study_get_test.pytests/routers/study_post_test.pytests/routers/tag_test_helper.pytests/routers/task_get_test.pytests/routers/task_list_test.pytests/routers/task_tag_test.pytests/routers/task_type_get_test.pytests/routers/task_type_list_test.pytests/routers/users_delete_test.pytests/types_test.py
💤 Files with no reviewable changes (2)
- src/routers/openml/init.py
- src/schemas/datasets/init.py
|
Patch coverage is a false positive, it's flagging a missing condition which was also missing before, but is now flagged because a renamed function. No code was altered in this PR, other than the removal of some dead code and moving function and classes around as-is. |
1 participant
Make some non-functional changes to the code base to try and make it more consistent/easier to navigate.
Mainly: