Skip to content

BUG: Compute label bounding boxes the same way with or without SciPy - #135

Open
demianw wants to merge 1 commit into
masterfrom
numpy-label-bounding-boxes
Open

demianw wants to merge 1 commit into
masterfrom
numpy-label-bounding-boxes

Conversation

@demianw

@demianw demianw commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

What was wrong

compute_label_bounding_boxes had two code paths (see #100):

  • with SciPy installed, ndimage.find_objects slices were mapped to RAS with their exclusive stop, so every box was one voxel larger along each image axis; it also transformed only the two corner points, which is wrong for an affine that is not diagonal;
  • without SciPy, the box spans the RAS coordinates of the label's voxel centres (this is what CI tests, as SciPy is not installed there). This path scanned the whole image once per label.

The relative terms (anterior_of, superior_of, medial_of, ...) therefore depended on an undeclared optional dependency.

The change

A single NumPy implementation with the semantics of the former fallback: the box of a label spans the RAS coordinates of its voxel centres. Non-zero voxels are grouped by label with one sort and np.minimum.reduceat/np.maximum.reduceat. Label keys are plain int. The SciPy branch is removed.

Behaviour change

Users who had SciPy installed will get slightly different results for relative terms: their label boxes shrink by one voxel on one side per axis. On the test atlas (IIT3mean_desikan_2009.nii.gz, 1 mm voxels, affine x axis negative) the SciPy boxes were exactly 1 mm larger on the left, anterior and superior sides of every one of the 183 labels; the other three sides were identical.

Measured on the test dataset (6783 tracts):

  • The three test-dataset query files give identical tract sets with the old SciPy path, the old fallback (SciPy hidden) and the new code: cortico_spinal.left 385/385/385, cortico_spinal.right 0/0/0, uncinate.left 102/102/102 (it uses anterior_of(amygdala.side)), uncinate.right 0/0/0, all_commissural 3/3/3.
  • Evaluating each single relative term on each of the 183 labels, SciPy path vs new code: anterior_of differs for 165 labels (median 41, max 138 tracts), superior_of for 165 labels (median 69, max 138), lateral_of.left/medial_of.right for 92 labels (median 88, max 382); posterior_of, inferior_of, medial_of.left/lateral_of.right are unchanged. So whole queries that combine a relative term with other terms may or may not change; single relative terms on their own do.
  • Results for users without SciPy do not change (the new boxes equal the old fallback's for all labels).

Speed of compute_label_bounding_boxes alone on the test atlas (182×218×182, 183 labels, min of 3): old SciPy path 0.10 s, old fallback 9.0 s, new 0.35 s.

Testing

  • New tract_querier/tests/test_label_bounding_boxes.py: the example of BUG: label bounding boxes differ depending on whether SciPy is installed #100 (affine diag(-2, 2, 2), expected [-6, 4, 4, -4, 6, 6]); a random atlas with negative labels and a non-diagonal affine compared with per-label min/max of the voxel coordinates; an all-zero image. With SciPy installed, the first two fail on master.
  • The comparisons above (scripts run locally against master with and without SciPy, and this branch).
  • python -m pytest -q -p no:cacheprovider tract_querier (with SciPy installed in the venv for the comparison): 45 passed, 1 skipped.

Merge order

Independent of #122 and #129 (a trial merge with them has no conflict). #140 (#117, same file) is meant to be merged after this one.

Closes #100

🤖 Generated with Claude Code

compute_label_bounding_boxes used scipy.ndimage.find_objects when SciPy
was installed and per-label voxel coordinates otherwise. The SciPy path
mapped the exclusive slice stop to RAS, so its boxes were one voxel
larger along each image axis, and it transformed only two corners, which
is wrong for affines that are not diagonal. Relative terms selected
different tracts depending on an undeclared optional dependency.

Keep a single NumPy implementation with the semantics of the fallback,
which is the one tested in CI: the box of a label spans the RAS
coordinates of its voxel centres. Group the voxels by label with one
sort instead of scanning the image once per label.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BUG: label bounding boxes differ depending on whether SciPy is installed

1 participant