Skip to content

[ENH/FIX] Add flag to turn off clipping during cleaning when clip_edges is True#201

Open
36000 wants to merge 2 commits into
tractometry:mainfrom
36000:allow_fullcleaning_clipping
Open

[ENH/FIX] Add flag to turn off clipping during cleaning when clip_edges is True#201
36000 wants to merge 2 commits into
tractometry:mainfrom
36000:allow_fullcleaning_clipping

Conversation

@36000

@36000 36000 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

This also includes a fix to make the clipping logic for all of our cleaning methods the same (standard, orientation, quickbundles, isolation forest) .

Copilot AI review requested due to automatic review settings July 8, 2026 22:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new clean_unclipped flag to control whether cleaning steps operate on unclipped streamlines when clip_edges=True, aiming to standardize clipping behavior across multiple cleaning criteria (standard/Mahalanobis, orientation, QuickBundles thresholding, isolation forest).

Changes:

  • Add clean_unclipped parameter to the public recognize() API and propagate it into bundle recognition.
  • Refactor cleaning criteria to use a unified “clip for cleaning” decision.
  • Update recognition test coverage to exercise the new flag when clip_edges=True.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
AFQ/recognition/tests/test_recognition.py Updates clip_edges API test to pass the new clean_unclipped flag.
AFQ/recognition/recognize.py Adds clean_unclipped argument and forwards it to recognize_bundles.
AFQ/recognition/criteria.py Refactors cleaning criteria to accept a “clip for cleaning” decision (currently introduces runtime issues that need fixes).
Comments suppressed due to low confidence (1)

AFQ/recognition/recognize.py:32

  • clean_unclipped is inserted into the middle of the recognize positional-parameter list (between clip_edges and rb_recognize_params). This is a backwards-incompatible API change for any downstream code that passes optional args positionally (it will silently shift arguments). To avoid breaking positional callers, add the new argument at the end of the signature (or make optional args keyword-only) and keep existing positional ordering intact.
def recognize(
    tg,
    img,
    mapping,
    bundle_dict,
    reg_template,
    nb_points=False,
    nb_streamlines=False,
    clip_edges=False,
    clean_unclipped=False,
    rb_recognize_params=None,
    refine_reco=False,
    prob_threshold=0,
    dist_to_waypoint=None,

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

Comment thread AFQ/recognition/criteria.py
Comment thread AFQ/recognition/criteria.py
Comment thread AFQ/recognition/criteria.py
Comment thread AFQ/recognition/criteria.py
Comment thread AFQ/recognition/criteria.py
@36000 36000 force-pushed the allow_fullcleaning_clipping branch from 211ec7f to 2ac430d Compare July 8, 2026 22:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread AFQ/recognition/criteria.py Outdated
Comment thread AFQ/recognition/recognize.py
Comment thread AFQ/recognition/tests/test_recognition.py
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.

2 participants