Skip to content

Add hole filling option to mask2stl#15

Merged
crnh merged 5 commits intomainfrom
crnh/feat/hole-filling
Mar 5, 2026
Merged

Add hole filling option to mask2stl#15
crnh merged 5 commits intomainfrom
crnh/feat/hole-filling

Conversation

@crnh
Copy link
Member

@crnh crnh commented Mar 5, 2026

Add --fill-holes flag to fill holes in the binary mask before converting it to a mesh.
I also added basic tests for the results of mask2stl.

@crnh crnh self-assigned this Mar 5, 2026
Copilot AI review requested due to automatic review settings March 5, 2026 12:00
@crnh crnh requested a review from jwmbeenakker as a code owner March 5, 2026 12:00
@crnh crnh added the enhancement New feature or request label Mar 5, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an optional hole-filling step to the mask2stl conversion pipeline and introduces basic integration tests (using committed test data) to validate mesh output, alongside some Ruff configuration refactoring.

Changes:

  • Add fill_holes_in_mask() and a --fill-holes/-f CLI flag to optionally fill holes in binary masks before meshing.
  • Add integration tests and _data fixtures to compare generated meshes against expected STL outputs.
  • Introduce ruff_defaults.toml and wire it into pyproject.toml; update README with the new CLI option.

Reviewed changes

Copilot reviewed 6 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/stl2mask/mask2stl.py Implements hole-filling + CLI flag wiring; updates mask2stl signature.
tests/test_mask2stl.py Adds mesh-based integration tests and updates existing unit tests for new parameter.
tests/conftest.py Adds a data_path fixture for locating tests/_data.
tests/_data/sphere.nii.gz Adds binary mask fixture for sphere conversion tests.
tests/_data/hollow_sphere.nii.gz Adds binary mask fixture for hollow-sphere conversion tests.
README.md Documents --fill-holes option.
ruff_defaults.toml Adds shared Ruff defaults/config presets.
pyproject.toml Configures Ruff to extend ruff_defaults.toml and adds a Hatch static-analysis env.
Comments suppressed due to low confidence (2)

src/stl2mask/mask2stl.py:44

  • fill_holes_in_mask computes foreground_value by converting the whole image to a NumPy view and taking .max(), which adds another full pass over the volume. Since mask2stl already computes mask_values for validation, consider reusing mask_values.max() (or passing the foreground value in) to avoid the extra scan when --fill-holes is enabled.
    foreground_value = float(sitk.GetArrayViewFromImage(mask).max())

    return sitk.BinaryFillhole(mask, fullyConnected=False, foregroundValue=foreground_value)

tests/test_mask2stl.py:275

  • The comment says hole filling is enabled, but the call passes fill_holes=False. Please fix the comment (or the argument) so the test intent matches the behavior being exercised.
    # Convert the mask to STL with hole filling enabled
    mask2stl_module.mask2stl(mask_path=mask_path, image_path=None, output_path=output_path, fill_holes=False)


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@jwmbeenakker jwmbeenakker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦷

@crnh crnh merged commit 0c58e90 into main Mar 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants