BrainLesion / BrainLesion/BraTS

Fix MissingMRI output errors, preserve Loguru handlers, and clarify preprocessing dispatch

Open
#163 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
85
Forks
19
Avg merge
13h 22m
Merged PRs (30d)
5

Description

### Summary

Two user-facing bugs are confirmed:

1. MissingMRI output processing raises `StopIteration` when the container produces no output.
2. Importing `brats` removes all globally configured Loguru handlers.

The preprocessing router also needs clarification and stronger tests. Its current enum-class-name string checks work for the supported `Algorithms` enum members, but rely on Enum string representation and silently fall back to SRI24 when given a raw value such as `challenge.value`. Raw values are also ambiguous because different challenge enums reuse identifiers such as `BraTS25_1`.

### 1. Make preprocessing dispatch explicit

`preprocess_for_challenge` currently derives:

```python
challenge_name = str(challenge)
```

and dispatches by checking whether an enum class name appears in that string.

For correctly typed enum members this currently works, but it is implicit and brittle. Passing a raw string or `.value` does not identify the challenge family and falls through to the default SRI24 pipeline.

Use explicit `isinstance` checks for the supported challenge enum classes. Consider rejecting non-enum inputs rather than silently selecting the default pipeline.

BraTS 2025+ metastases additionally require the native-space preprocessing path. This specific route is being addressed separately in PR #174.

### 2. Raise a useful error for missing MissingMRI output

These expressions raise `StopIteration` when no output exists:

```python
Path(tmp_output_folder).iterdir().__next__()
Path(tmp_output_folder).glob(f"*{internal_name}*").__next__()
```

Raise a descriptive `FileNotFoundError` instead. Add tests for both single-subject and batch inference.

### 3. Preserve user Loguru configuration

`brats/__init__.py` calls:

```python
logger.remove()
```

This removes handlers configured by the application before `import brats`.

Use the existing package-level logging disable mechanism instead:

```python
logger.disable("brats")
```

Add a test proving that a user-installed handler survives importing `brats`.

### Scope

The following are intentionally deferred:

- Removing the `only_t1c` parameter from the base segmentation method.
- Renaming the `algorithm` constructor parameter or `self.algorithm` attribute.
- Replacing the hardcoded `(240, 240, 155)` shape with a named constant.

Contributor guide

Open the contributing guide

Research direction

Start with preprocess_for_challenge and the MissingMRI output lookups shown in the issue, then inspect brats/__init__.py and its logger setup. Add coverage for explicit enum dispatch, missing output in single-subject and batch inference, and preservation of a user-installed handler. Done means descriptive FileNotFoundError errors, unchanged handlers after import, and no silent raw-value fallback; the BraTS 2025+ native-space route is deferred to PR #174.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
machine-learning, testing, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.