Add spatial_dims parameter to ROIFilter for dense data support#284
Add spatial_dims parameter to ROIFilter for dense data support#284SimonHeybrock merged 3 commits intomainfrom
Conversation
ROIFilter now accepts an explicit spatial_dims parameter to control which dimensions get flattened when applying the filter. This enables support for dense data arrays like (time, x, y) where the time dimension should be preserved while only the spatial dimensions are flattened. For projections where indices represent a subset of the detector (e.g., a z=0 slice), spatial_dims should be set to the full detector dimensions. RollingDetectorView.make_roi_filter() now passes detector_number.dims. Prompt: We need to look into support dense data arrays instead of event-data in RollingDetectorView and ROIFilter. Let us talk about the latter first. Do you think the current approach can be tweaked slightly, or is it too inefficient? Our dense data would be something like (time,x,y) where the selection is in the (x,y), detector_number labelling pixels there. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
| *, | ||
| selection: sc.Variable, | ||
| norm: float = 1.0, | ||
| spatial_dims: tuple[str, ...] | None = None, |
There was a problem hiding this comment.
Can 'spatial_dims' be confusing?
Say you have a 3d cube of pixels, and you do a 2d xy projection, but then the roi is supposed to show the profile along z. All 3 dims xyz are spatial.
In addition, in the docstring, you explain
For dense data like (time, x, y), pass ('x', 'y') to preserve time.
Is it then not easier for users to reverse the logic and call the argument dims_to_keep or dims_to_preserve?
There was a problem hiding this comment.
We usually just pass spatial_dims=detector_number.dims so the reverse does not feel simpler. And to you first question: As the docstring you explains you in fact need to pass all the spatial dims, not just those left after a projection/slicing.
There was a problem hiding this comment.
I mean the docstring in ROIFilter:
spatial_dims:
Dimensions of the detector that should be flattened when applying the
filter. If None, defaults to indices.dims. For projections where indices
represent a subset of the detector, this should be set to the full
detector dimensions.
There was a problem hiding this comment.
I didn't really get why reversing the logic is difficult, but I also don't really mind that much.
Do as you prefer, it was just a suggestion.
3 participants
Summary
ROIFilternow accepts an explicitspatial_dimsparameter to control which dimensions get flattened when applying the filter(time, x, y)where the time dimension should be preserved while only the spatial dimensions are flattenedRollingDetectorView.make_roi_filter()now passesdetector_number.dimsasspatial_dimsFor projections where indices represent a subset of the detector (e.g., a z=0 slice),
spatial_dimsshould be set to the full detector dimensions rather than being inferred fromindices.dims.Test plan
(time, x, y)preserves time dimensionspatial_dimsfor projection cases🤖 Generated with Claude Code