Conversation
There was a problem hiding this comment.
Pull request overview
This PR appears to add support for “reward bundles” by introducing new template assets and enabling more flexible ROI handling (including mixed-space ROI definitions and ROI dilation behavior used in recognition/tracking workflows).
Changes:
- Add MassP template fetching/loading utilities and update bibliography references.
- Extend BundleDict ROI handling to support mixed-space ROI specifications and update ROI transformation call sites to pass images (not affines).
- Enhance
RoiImagegeneration with optional ROI dilation and WM-only masking.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/source/references.bib | Adds two new literature references. |
| AFQ/tasks/utils.py | Adjusts base filename generation to avoid unconditional trailing-char stripping. |
| AFQ/tasks/mapping.py | Updates transform_rois call to pass the DWI image (aligning with new API). |
| AFQ/tasks/decorators.py | Removes forced logger level configuration. |
| AFQ/recognition/utils.py | Introduces shared tolerance_mm_to_vox utility. |
| AFQ/recognition/preprocess.py | Refactors to delegate tolerance conversion to AFQ.recognition.utils. |
| AFQ/recognition/criteria.py | Updates ROI transformation call to pass the image instead of the affine. |
| AFQ/nn/synthseg.py | Updates label groupings used to derive PVE outputs. |
| AFQ/definitions/image.py | Adds ROI dilation + WM-only masking options for ROI-derived images. |
| AFQ/data/fetch.py | Adds MassP template fetcher/reader utilities. |
| AFQ/api/group.py | Removes logger level forcing; docstring example was modified. |
| AFQ/api/bundle_dict.py | Adds mixed-space ROI support and updates transformation helper signature to use an image object. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if isinstance(roi_or_sl, dict): | ||
| space = roi_or_sl.get("space", None) | ||
| roi_or_sl = roi_or_sl.get("roi", None) | ||
| if roi_or_sl is None or space is None: | ||
| raise ValueError( | ||
| ( | ||
| f"Unclear ROI definition for {roi_or_sl}. " | ||
| "See 'Defining Custom Bundle Dictionaries' " | ||
| "in the documentation for details." | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This introduces a new ROI definition format (dicts containing roi + space) and new error-paths for invalid definitions. There are existing BundleDict tests (e.g., AFQ/tests/test_bundle_dict.py), but none appear to cover this new format. Please add tests for: (1) valid space="template"|"subject" ROI dicts, (2) invalid/missing keys raising the intended ValueError, and (3) mixed-space bundles.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
1 participant
Uh oh!
There was an error while loading. Please reload this page.