improve image normalization capabilities#372
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens and extends image normalization/downsampling utilities by improving dtype handling, adding validation options, fixing edge cases (divide-by-zero, padding semantics, cutoff handling), and expanding unit test coverage.
Changes:
- Added
value_range_normalizationwith range validation,out_dtype, andreturn_floatsupport. - Fixed dtype detection in
percentile_normalization, cutoff semantics inrescale_image, and edge cases in downsampling (zero max, 2D/3D padding + logging). - Updated
rolling_window_meanto avoid NaNs on constant arrays and added tests for new behaviors.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/unit_tests/processing/test_images.py | Adds unit tests covering new normalization behaviors and downsampling/rescaling edge cases. |
| src/scportrait/processing/images/_image_processing.py | Implements normalization API additions and fixes multiple edge cases; replaces prints with logging. |
| src/scportrait/processing/images/init.py | Exposes value_range_normalization at the package level. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if cutoff_threshold is not None: | ||
| if img.max() > cutoff_threshold: | ||
| _img = img.copy() | ||
| values = _img[_img < (cutoff_threshold / factor)] | ||
| values = _img[_img < cutoff_threshold] | ||
| else: | ||
| values = img |
There was a problem hiding this comment.
values is undefined when cutoff_threshold is not None and img.max() <= cutoff_threshold, causing an UnboundLocalError at the percentile calculation. Initialize values = img within the cutoff_threshold is not None branch (or add an else for the inner if) so values is always defined.
use keyword arguments instead of positional arguments Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…for vectorization
…/scPortrait into add_image_normalization
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…x any failing tests
…n visualization functions, make vis a hidden module, cleanup doc imports
for more information, see https://pre-commit.ci
…/scPortrait into add_image_normalization
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/scportrait/plotting/_vis.py:45
plot_image_arraydocstring says it returnsNone, but the function returns aFigurewhenreturn_fig=True. Update the docstring to documentreturn_figand the actual return type/behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Key Changes
percentile_normalizationnow correctly detects floating dtypes.value_range_normalizationgains validation, out_dtype, and return_float options.downsample_img_paddinghandles 2D/3D padding correctly and uses logging.rescale_imagevalidates dtype and uses raw cutoff_threshold.rolling_window_meanupdated to be numba‑compatible and avoids NaNs on constant arrays.