Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces automatic mask padding functionality to the blurring mask calculation logic, allowing masks that are too small for a kernel footprint to be automatically padded rather than raising an exception. It also improves the robustness of k-nearest neighbors interpolation by adding fallback logic for different data grid attribute structures.
Changes:
- Added
allow_paddingparameter to blurring mask methods to support automatic padding when masks are too small for kernel footprints - Introduced
blurring_mask_required_shape_fromutility function to compute minimal required mask dimensions - Enhanced k-nearest neighbors interpolator with nested try-except blocks for more robust data grid attribute handling
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| autoarray/mask/mask_2d_util.py | Added new blurring_mask_required_shape_from function and updated blurring_mask_2d_from to support automatic symmetric padding with warnings |
| autoarray/mask/derive/mask_2d.py | Added allow_padding parameter to blurring_from method signature |
| autoarray/inversion/mesh/interpolator/knn.py | Added nested fallback logic to handle different data grid attribute structures (over_sampled.array, array, or direct grid) |
| autoarray/dataset/grids.py | Changed blurring property to use allow_padding=True for automatic mask padding during blurring grid computation |
Comments suppressed due to low confidence (1)
autoarray/mask/derive/mask_2d.py:213
- When
allow_padding=Trueand padding occurs, the returned Mask2D has a larger shape than the input mask but uses the samepixel_scalesandorigin. This creates a potential coordinate system misalignment: pixels in the padded mask correspond to different physical coordinates than pixels in the original mask. For example, if 1 pixel is added to each side, the original pixel (0,0) is now at position (1,1) in the padded mask, and the new pixel (0,0) represents physical space that was outside the original mask. Consider whether this is the intended behavior, and if so, document it clearly. If not, the origin may need to be adjusted by (-pad_top * pixel_scales[0], -pad_left * pixel_scales[1]) to maintain coordinate alignment.
return Mask2D(
mask=blurring_mask,
pixel_scales=self.mask.pixel_scales,
origin=self.mask.origin,
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def blurring_from( | ||
| self, kernel_shape_native: Tuple[int, int], allow_padding: bool = False | ||
| ) -> Mask2D: |
There was a problem hiding this comment.
The allow_padding parameter is missing from the Parameters section of the docstring. This parameter should be documented to explain its purpose and behavior to users of this method.
| import warnings | ||
| from typing import Tuple | ||
|
|
||
| import numpy as np | ||
| from scipy.ndimage import binary_dilation | ||
|
|
||
|
|
||
| def required_shape_for_kernel( | ||
| mask_2d: np.ndarray, | ||
| kernel_shape_native: Tuple[int, int], | ||
| ) -> Tuple[int, int]: | ||
| """ | ||
| Return the minimal shape the mask must be padded to so that a kernel with the given | ||
| footprint can be applied without sampling beyond the array edge, while preserving | ||
| parity (odd->odd, even->even) in each dimension. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| mask_2d | ||
| 2D boolean array where False is unmasked and True is masked. | ||
| kernel_shape_native | ||
| (ky, kx) footprint of the convolution kernel. | ||
|
|
||
| Returns | ||
| ------- | ||
| required_shape | ||
| The minimal (ny, nx) shape such that the minimum distance from any unmasked | ||
| pixel to the array edge is at least (ky//2, kx//2), and each dimension keeps | ||
| the same parity as the input mask. | ||
| """ | ||
| mask_2d = np.asarray(mask_2d, dtype=bool) | ||
|
|
||
| ky, kx = kernel_shape_native | ||
| if ky <= 0 or kx <= 0: | ||
| raise ValueError( | ||
| f"kernel_shape_native must be positive, got {kernel_shape_native}." |
There was a problem hiding this comment.
The function will raise a generic ValueError from min_false_distance_to_edge if the input mask contains no unmasked pixels (all True values). Consider catching this ValueError and raising a more context-specific exception (like MaskException) with a clearer error message that explains the issue in terms of blurring mask calculation.
| The returned blurring mask is a *mask* where the blurring-region pixels are | ||
| unmasked (False) and all other pixels are masked (True). | ||
|
|
||
| If the input mask is too small for the kernel footprint: | ||
| - allow_padding=False (default): raises an exception. | ||
| - allow_padding=True: pads the mask symmetrically with masked pixels (True) to the | ||
| minimal required shape (with parity preserved) and emits a warning. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| mask_2d | ||
| 2D boolean mask. | ||
| kernel_shape_native | ||
| (ky, kx) kernel footprint. | ||
| allow_padding | ||
| If False, raise if padding is required. If True, pad and warn. | ||
|
|
||
| Returns | ||
| ------- | ||
| blurring_mask | ||
| Boolean mask of the same shape as the (possibly padded) input. | ||
| """ | ||
| mask_2d = np.asarray(mask_2d, dtype=bool) | ||
|
|
||
| ky, kx = kernel_shape_native | ||
| if ky <= 0 or kx <= 0: | ||
| raise ValueError( | ||
| f"kernel_shape_native must be positive, got {kernel_shape_native}." | ||
| required_shape = required_shape_for_kernel(mask_2d, kernel_shape_native) | ||
|
|
||
| if required_shape != mask_2d.shape: | ||
| if not allow_padding: | ||
| raise exc.MaskException( |
There was a problem hiding this comment.
The new allow_padding functionality is not covered by tests. Consider adding test cases that verify: 1) padding occurs correctly when the mask is too small and allow_padding=True, 2) the warning is emitted, 3) the padding amounts are calculated correctly for both even and odd differences, and 4) the resulting blurring mask has the expected shape and values after padding.
This pull request introduces improvements to the blurring mask calculation logic, primarily by adding support for automatic mask padding when the mask is too small for the kernel footprint. It also enhances the robustness of data grid handling in the k-nearest neighbors interpolator. The most important changes are grouped below.
Blurring mask calculation and padding enhancements
allow_paddingparameter to theblurring_frommethod inMask2Dand its underlying utility function, allowing the mask to be automatically padded with masked pixels if it is too small for the kernel footprint. This prevents errors and improves usability when performing convolutions near the edge of arrays. [1] [2] [3] [4]blurring_mask_required_shape_fromutility function, which computes the minimal required shape for a mask so that a kernel can be applied without sampling beyond the array edge.blurring_mask_2d_fromfunction to use the new required shape logic and to perform symmetric padding of the mask when needed, emitting a warning if padding occurs.Robustness improvements
_mappings_sizes_weightsmethod in the k-nearest neighbors interpolator to more robustly handle different possible data grid attribute structures, falling back gracefully if expected attributes are missing.