Re: Possibly reduce duplication between `signal` and `ndimage` filtering routines

Just to close the loop, here's a PR: https://github.com/scipy/scipy/pull/18341
Like it says in the PR, I am actually not 100% positive our test suite
is exhaustive, so if someone sees a problem with this, please respond
here or on the PR.

Evgeni

On Wed, Feb 22, 2023 at 2:42 PM Ralf Gommers <[email protected]> wrote:
>
>
>
> On Tue, Feb 21, 2023 at 8:12 PM Evgeni Burovski <[email protected]> wrote:
>>
>> Hi,
>>
>> In `scipy.signal` and `scipy.ndimage`, we ship a set of
>> almost-if-not-completely overlapping functionality. For instance,
>> - `signal.order_filter` has a counterpart, `ndimage.rank_filter`;
>> - `signal.median_filter` can be implemented in terms of the rank filter
>>
>> In fact, applying the patch below (the patch is based on the CuPy
>> implementation of `cupyx.signal`, see
>> https://github.com/cupy/cupy/blob/v11.5.0/cupyx/scipy/signal/_signaltools.py#L490),
>> and running `$ python dev.py test -s signal`, I only get three
>> failures in `medfilt`, related to
>> - object arrays of `Decimal` instances
>> - `longdouble` arrays.
>>
>> So, assuming we can deprecate longdoubles and decimals in these
>> filters, we could potentially remove some 700 lines of obscure C code
>> in https://github.com/scipy/scipy/blob/main/scipy/signal/_lfilter.c.in
>>
>> Thoughts?
>
>
> +1 this sounds like a healthy idea. There's a lot of duplication in these types of routines, and it's quite under-maintained code. And these deprecations sound perfectly fine.
>
> Cheers,
> Ralf
>
>
>>
>> Evgeni
>>
>>
>> P.S. The patch to check the mapping between
>> signal.{order,median}_filter and ndimage.rank_filter:
>>
>>
>> ```diff
>> diff --git a/scipy/signal/_signaltools.py b/scipy/signal/_signaltools.py
>> index 31baba2421..2f00ead3a2 100644
>> --- a/scipy/signal/_signaltools.py
>> +++ b/scipy/signal/_signaltools.py
>> @@ -1492,7 +1492,15 @@ def order_filter(a, domain, rank):
>>          if (dimsize % 2) != 1:
>>              raise ValueError("Each dimension of domain argument "
>>                               "should have an odd number of elements.")
>> -    return _sigtools._order_filterND(a, domain, rank)
>> +
>> +    result = _sigtools._order_filterND(a, domain, rank)
>> +
>> +    import scipy.ndimage as ndimage
>> +    import numpy.testing as testing
>> +    res_ndi = ndimage.rank_filter(a, rank, footprint=domain, mode='constant')
>> +    testing.assert_equal(result, res_ndi)
>> +
>> +    return result
>>
>>
>>  def medfilt(volume, kernel_size=None):
>> @@ -1555,7 +1563,17 @@ def medfilt(volume, kernel_size=None):
>>
>>      numels = np.prod(kernel_size, axis=0)
>>      order = numels // 2
>> -    return _sigtools._order_filterND(volume, domain, order)
>> +    result = _sigtools._order_filterND(volume, domain, order)
>> +
>> +
>> +    import scipy.ndimage as ndimage
>> +    import numpy.testing as testing
>> +    size = np.prod(kernel_size)
>> +    res_ndi = ndimage.rank_filter(volume, size // 2, size=kernel_size,
>> +                                mode='constant')
>> +    testing.assert_equal(result, res_ndi)
>> +
>> +    return result
>>
>>
>>  def wiener(im, mysize=None, noise=None):
>> ```
>> _______________________________________________
>> SciPy-Dev mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
>> https://mail.python.org/mailman3/lists/scipy-dev.python.org/
>> Member address: [email protected]
>
> _______________________________________________
> SciPy-Dev mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
> https://mail.python.org/mailman3/lists/scipy-dev.python.org/
> Member address: [email protected]
_______________________________________________
SciPy-Dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://mail.python.org/mailman3/lists/scipy-dev.python.org/
Member address: [email protected]