Skip to content

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Sep 12, 2024

Reference issue

A series of previous MRs (#18016, #18261, #18305, #20809, #20811) addes axes support to ndimage functions in _filters.py.

This MR extends the same axes support to most functions in _morphology.py. All grayscale and binary morphology functions are covered, only the three distance_transform_* functions are not updated here.

The changes needed to support axes were relatively small as all of these functions are based on either (_filters._min_or_max_filter or _binary_erosion). Given that _min_or_max_filter was already updated to support axes previously, _binary_erosion is the main function updated here.

cc: @ev-br, @lucascolley, @mdhaber and @dschmitz89 who had helped review some of the prior MRs.

What does this implement/fix?

The functions updated in this MR fall into two groups:

  1. The grayscale morphology functions are based on calling _filters._min_or_max_filter which already has axes support, so those were trivial to update here. These include grey_dilation, grey_erosion, grey_opening, grey_closing, morphological_laplace, morphological_gradient, white_tophat and black_tophat.

  2. The binary morphology functions are all based on _binary_erosion, which has axes support added to it in this MR. These include binary_erosion, binary_dilation, binary_opening, binary_closing, binary_hit_or_miss, binary_propagation and binary_fill_holes.

Additional information

The testing approach follows that taken in the other recent MRs adding axes support to the filtering functions.

@grlee77 grlee77 added enhancement A new feature or improvement scipy.ndimage labels Sep 12, 2024
@grlee77 grlee77 self-assigned this Sep 12, 2024
@grlee77 grlee77 force-pushed the ndimage-morphology-axes branch from c87e27c to 6801608 Compare September 14, 2024 17:26
@grlee77
Copy link
Contributor Author

grlee77 commented Sep 16, 2024

@lucascolley : The Array API case is happy now.

There was one seemingly unrelated failure in the "Oldest GCC" case, but I am rerunning that one now to see if it is reproducible

@ev-br
Copy link
Member

ev-br commented Sep 21, 2024

assert_array_almost_equal in this test file (and elsewhere in ndimage on main) is a thin wrapper of xp_assert_close already: https://github.com/scipy/scipy/blob/main/scipy/ndimage/tests/test_morphology.py#L4

@ev-br
Copy link
Member

ev-br commented Nov 17, 2024

Where does this PR stand @grlee77?

@skip_xp_backends("cupy",
reasons=["these filters do not yet have axes support"],
cpu_only=True,
exceptions=['cupy', 'jax.numpy'])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, not this pattern again :-).
Now that skip_xp_backends decorators can be stacked, we can express things is a less confusing way. So these tests need to skip all non-cpu backends, and additionally you skip cupy and jax.numpy on CPU, is this correct?
Then,

@skip_xp_backends(cpu_only)
@skip_xp_backends("cupy", reason="these filters do not yet have axes support in CuPy")
@skip_xp_backends("jax.numpy", reason="these filters do not yet have axes support in JAX.numpy")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I will update it

@ev-br ev-br added this to the 1.15.0 milestone Nov 17, 2024
@ev-br ev-br merged commit fb625cb into scipy:main Nov 23, 2024
36 of 37 checks passed
@ev-br
Copy link
Member

ev-br commented Nov 23, 2024

CI is green modulo an unrelated failure in lapack wrappers on MacOS / OpenBLAS, let's land this. Thank you @grlee77 , all.

@ev-br ev-br added the needs-release-note a maintainer should add a release note written by a reviewer/author to the wiki label Nov 24, 2024
@ev-br
Copy link
Member

ev-br commented Nov 24, 2024

@grlee77 mind adding a release note snippet for the upcoming 1.15 release please?
https://github.com/scipy/scipy/wiki/*-Release-Note-Entries-for-SciPy-1.15.0-(asterisk-makes-this-appear-at-the-top)

@grlee77
Copy link
Contributor Author

grlee77 commented Nov 29, 2024

@grlee77 mind adding a release note snippet for the upcoming 1.15 release please?

Done. Thanks @ev-br, @lucascolley and @tylerjereddy for reviewing

@lucascolley lucascolley removed the needs-release-note a maintainer should add a release note written by a reviewer/author to the wiki label Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.ndimage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants