fix: use P-axis length for ys calculation and clip S-axis bounds (#167)

[rtmalikian]

Summary

Fixes the second part of #167: incorrect ys axis calculation and missing bounds clipping in QuickShear defacing.

Root Cause

The ys calculation in run_quickshear() used mask_RPS.shape[2] (S-axis length) instead of mask_RPS.shape[1] (P-axis length):

# Before (buggy)
ys = np.arange(0, mask_RPS.shape[2]) * slope + yint

In RPS orientation, the defacing loop iterates over P-axis positions (axis 1) and computes S-axis limits (axis 2) for each. Using shape[2] caused:

1. Incorrect defacing masks for images with narrow S axes (e.g., shape (666, 512, 30)) — the loop only iterated over 30 positions instead of 512
2. IndexError when computed S-axis limits exceeded shape[2] bounds (defaced_mask_RPS[:, x, :y] where y > shape[2])

Fix

# After (fixed)
ys = np.arange(0, mask_RPS.shape[1]) * slope + yint
# ...
for x, y in zip(np.nonzero(ys > 0)[0], ys.astype(int)):
    y_clipped = min(y, mask_RPS.shape[2])  # prevent out-of-range
    defaced_mask_RPS[:, x, :y_clipped] = 0

Verification

Tested with 4 scenarios including the exact shapes from the bug report:

• Normal brain mask (centered) ✓
• Brain touching P-axis boundary ✓
• Narrow S-axis (shape[2]=5) ✓
• Large P-axis / narrow S-axis (512 × 180) — the IndexError case ✓

All tests pass with no IndexError and correct defacing behavior.

---

About the Author: Raphael Malikian — Clinical AI Solutions Architect. I specialise in building and fixing AI/ML systems for healthcare, including vector databases, RAG pipelines, and clinical NLP. If you need help with your project or think I can add value to your organisation, feel free to reach out — I'd love to connect.

📧 rtmalikian@gmail.com
🔗 GitHub: https://github.com/rtmalikian
🔗 LinkedIn: http://www.linkedin.com/in/raphael-t-malikian-mbbs-bsc-hons-71075436a

---

Disclosure: This code was developed with assistance from mimo-2.5-pro (Xiaomi) via Hermes Agent (Nous Research). All changes were reviewed, tested against the actual codebase, and verified for correctness.

[Copilot]

Pull request overview

This pull request fixes an indexing bug in the QuickShear defacing implementation by computing the defacing-plane ys values across the correct axis (P-axis) and clipping S-axis slice bounds to prevent out-of-range writes.

Changes:

• Compute ys using mask_RPS.shape[1] (P-axis length) instead of mask_RPS.shape[2] (S-axis length).
• Clip computed S-axis limits (y) to mask_RPS.shape[2] before slicing to avoid out-of-bounds access.

---

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

[neuronflow]

@uturkbey @nicmuenster do you have time to look into this?

[MarcelRosier]

im not familiar with the details; if this should be a valid fix, it should also be raised in the reference implementation: https://github.com/nipy/quickshear/blob/master/quickshear.py

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

brainles_preprocessing/defacing/quickshear/nipy_quickshear.py:163

• The loop pairs x indices where ys > 0 with unfiltered ys.astype(int) values. If ys is not positive from index 0 onward, zip(np.nonzero(ys > 0)[0], ys.astype(int)) will misalign x and y (e.g., first positive x will be paired with ys[0]), producing an incorrect defacing mask. Filter ys by the same indices (or index into ys inside the loop) so each x uses its corresponding y.

    for x, y in zip(np.nonzero(ys > 0)[0], ys.astype(int)):
        y_clipped = min(y, mask_RPS.shape[2])
        defaced_mask_RPS[:, x, :y_clipped] = 0