[FIX] table generation with cluster corrected images in Jackknife and focuscounter#1087
Merged
Merged
Conversation
Contributor
Reviewer's GuideRefactors diagnostics threshold handling and cluster table generation for corrected images, adds scaffolded machine-learning API and specs for masked activation feature datasets, and updates tests/examples to cover the new behavior and future ML module. Sequence diagram for diagnostics cluster table generation with corrected imagessequenceDiagram
participant Diagnostics
participant MetaResult as result
participant Clusters as _get_clusters_table_and_label_maps
participant ClusterFunc as get_clusters_table
Diagnostics->>Clusters: _get_clusters_table_and_label_maps(result, target_img, target_image, target_threshold, cluster_threshold)
Clusters->>MetaResult: result.estimator
Clusters->>ClusterFunc: get_clusters_table(target_img, threshold, cluster_threshold, two_sided, return_label_maps=True)
ClusterFunc-->>Clusters: clusters_table, label_maps
alt [target_image is cluster_corrected and clusters_table not empty]
Clusters->>MetaResult: _get_peak_value_map_for_cluster_table(result, target_image)
Clusters->>MetaResult: get_map(peak_value_map, return_type=image)
Clusters->>ClusterFunc: get_clusters_table(masked_peak_img, 0, 0, two_sided)
ClusterFunc-->>Clusters: peak_clusters_table
Clusters-->>Diagnostics: peak_clusters_table, label_maps
else [not cluster_corrected or no clusters]
Clusters-->>Diagnostics: clusters_table, label_maps
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The cluster diagnostics helpers rely on fairly specific naming conventions (e.g.,
_level-cluster,_corr-,z_desc-*,stat_desc-*,stat_desc-group1MinusGroup2); consider centralizing or documenting these patterns in a single utility to reduce the risk of breakage if map naming changes elsewhere. - In
_get_clusters_table_and_label_maps, the logic for inferringtwo_sidedfrom the estimator or data is duplicated and slightly implicit; you might want to factor this into a dedicated helper so that future changes to the two-sided behavior are easier to maintain and test.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The cluster diagnostics helpers rely on fairly specific naming conventions (e.g., `_level-cluster`, `_corr-`, `z_desc-*`, `stat_desc-*`, `stat_desc-group1MinusGroup2`); consider centralizing or documenting these patterns in a single utility to reduce the risk of breakage if map naming changes elsewhere.
- In `_get_clusters_table_and_label_maps`, the logic for inferring `two_sided` from the estimator or data is duplicated and slightly implicit; you might want to factor this into a dedicated helper so that future changes to the two-sided behavior are easier to maintain and test.
## Individual Comments
### Comment 1
<location path="nimare/diagnostics.py" line_range="141-146" />
<code_context>
+ )
+
+
+def _get_cluster_support_data(label_maps, shape):
+ """Convert one or more cluster label maps to a binary support array."""
+ support = np.zeros(shape, dtype=bool)
+ for label_map in label_maps:
+ support |= np.asanyarray(label_map.dataobj) > 0
+ return support
+
</code_context>
<issue_to_address>
**suggestion:** Cluster support binary mask threshold may be too specific to positive labels.
The current `> 0` check assumes all cluster labels are positive. If labels ever include negative values with 0 as background, this will incorrectly exclude those clusters. Using `!= 0` would keep 0 as background while correctly including all non-zero labels and make the function more robust to different labeling schemes.
```suggestion
def _get_cluster_support_data(label_maps, shape):
"""Convert one or more cluster label maps to a binary support array."""
support = np.zeros(shape, dtype=bool)
for label_map in label_maps:
# Treat 0 as background and include all non-zero labels (positive or negative)
support |= np.asanyarray(label_map.dataobj) != 0
return support
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
jdkent
force-pushed
the
fix/table_generation
branch
from
July 9, 2026 21:21
798d4c9 to
c10e841
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1087 +/- ##
==========================================
+ Coverage 85.50% 85.53% +0.03%
==========================================
Files 56 56
Lines 11248 11300 +52
==========================================
+ Hits 9618 9666 +48
- Misses 1630 1634 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1086 .
Changes proposed in this pull request:
Summary by Sourcery
Introduce a new diagnostics threshold API and corrected-cluster handling, while scaffolding the future
nimare.mlmasked activation feature dataset module with tests, specs, and examples.Bug Fixes:
Enhancements:
target_thresholdparameter to diagnostics, deprecatingvoxel_threshwith explicit alias handling and validation.Documentation:
nimare.ml) module, including public API and scikit-learn compatibility contracts.examples/05_machine_learning.target_thresholdparameter and clarify its relationship to deprecatedvoxel_threshfor cluster-corrected images.Tests:
voxel_threshandtarget_thresholdin diagnostics.target_thresholdinstead ofvoxel_threshand validate tail label map naming, mixed-sign cluster warnings, and group-specific behavior.nimare.mlmodule, including a canonical Studyset fixture and placeholders asserting that unimplemented ML components currently raiseNotImplementedError.Chores:
nimare.mlmodule, exposingMAFeatureDataset,MAFeatureExtractor, andmake_map_reduceras placeholders.specs/001-ma-feature-dataset/, capturing research, implementation plan, tasks, and requirements.