Fix user-facing API bugs - #175
Open
MarcelRosier wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses several user-facing API issues in the BraTS orchestrator by making preprocessing dispatch stricter and explicit, improving MissingMRI failure behavior when container outputs are missing, and preventing import brats from wiping out user-configured Loguru handlers.
Changes:
- Updated
preprocess_for_challengeto dispatch via explicitisinstancechecks on supportedAlgorithmsenum subclasses and to reject raw (non-enum) inputs with aTypeError. - Replaced
StopIteration-style failures in MissingMRI output handling with descriptiveFileNotFoundErrors for both single and batch inference. - Adjusted package import-time logging behavior to preserve user Loguru handlers, and added/updated tests covering these behaviors.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
brats/preprocessing.py |
Makes preprocessing dispatch explicit by enum subclass and rejects non-enum inputs. |
brats/core/brats_algorithm.py |
Raises descriptive FileNotFoundError when MissingMRI outputs are absent. |
brats/__init__.py |
Disables brats namespace logging without removing user-installed Loguru handlers. |
tests/test_preprocessing.py |
Adds tests for preprocessing dispatch correctness and raw-value rejection. |
tests/core/test_missing_mri_algorithms.py |
Adds tests asserting MissingMRI inference raises FileNotFoundError on missing outputs. |
tests/utils/test_logging.py |
Adds a regression test ensuring user Loguru handlers survive import brats / reload. |
tests/utils/test_data_handling.py |
Enables brats logging in a test to ensure log files are non-empty under the new default-disable behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the user-facing API issues tracked in #163:
preprocess_for_challengeby concrete challenge-specificAlgorithmsenum subclasses and reject raw string values.FileNotFoundErrorexceptions for missing MRI outputs in MissingMRI inference.Verification
uv run --python 3.9 pytestuv run ruff check .uv run ruff format --check .mkdocs build --strictCloses #163