scipy icon indicating copy to clipboard operation
scipy copied to clipboard

ENH: NDInterpolator should support the 1D case

Open vnmabus opened this issue 2 years ago • 6 comments

Is your feature request related to a problem? Please describe.

As mentioned in #13432 it is natural to expect that the ND interpolators work for 1D too. If the code is different from N==1 and N>1, that should be an internal detail, and users shouldn't be expected to deal with that.

Describe the solution you'd like.

The ND interpolators should just work with 1D data.

Describe alternatives you've considered.

The current situation requires users to switch between different interpolators for 1D data and for ND data. This repeats efforts across codebases and increases the potential for errors.

Additional context (e.g. screenshots, GIFs)

No response

vnmabus avatar Jun 20 '22 11:06 vnmabus

Hi @vnmabus, thank you for reporting. I agree here ➕

tupui avatar Jun 20 '22 12:06 tupui

In the end of the day, I'm 99/100 certain that anyhow it boils down to a variation of

if ndim == 1:
    # use a 1D interpolator
else:
    ...  

My expectation is that this sort of logic is simpler in the user code because you can pick whatever 1D interpolator is most suitable for the specific use case. However if someone feels strongly enough to actually implement it, great, we'll review a pull request. So, PR welcome.

ev-br avatar Jun 23 '22 21:06 ev-br

I actually hit this at work today too, will try make a pr in the coming weeks

j-bowhay avatar Jun 30 '22 19:06 j-bowhay

Given that the original PR, https://github.com/scipy/scipy/pull/13432, was about linear and nearest scattered ND interpolators, and that the griddata code is literally (https://github.com/scipy/scipy/blob/main/scipy/interpolate/_ndgriddata.py#L240)

  if ndim == 1 and method in ('nearest', 'linear', 'cubic'):
        from ._interpolate import interp1d
        <... snip ...>
        ip = interp1d(points, values, kind=method, axis=0, bounds_error=False,
                      fill_value=fill_value)
        return ip(xi)
    elif method == 'nearest':
        ip = NearestNDInterpolator(points, values, rescale=rescale)
        return ip(xi)
    elif method == 'linear':
        ip = LinearNDInterpolator(points, values, fill_value=fill_value,
                                  rescale=rescale)
        return ip(xi)

it actually seems to me that adding an extra layer of if ndim == 1 to NearestNDInterpolator and LinearNDInterpolator is not warranted. Or is there a use case where griddata does not cut it?

cc @stefanv as the OP for gh-13432 (no good deed gets you not pinged, as it were :-))

ev-br avatar Jul 27 '22 10:07 ev-br

Or is there a use case where griddata does not cut it?

If you want an interpolator but don't know the points that you want to evaluate at ahead of time? With griddata you are forced into the overhead of rebuilding the interpolator every-time you have a new point you want to evaluate.

That being said it is a very easy workaround on the users side so on the fence to whether this is worth the effort.

j-bowhay avatar Aug 03 '22 21:08 j-bowhay

Makes sense! I'm with you on both counts; so the status here is then "triaged, waiting for a contributor": if somebody wants this enough to send a PR, we'll review.

ev-br avatar Aug 04 '22 05:08 ev-br

No activity for ~10 months. The feature is borderline: easy to work around on the user side, easy to implement, not clear if it's worth implementing. If somebody feels like sending a PR, we can review of course; whether we want to encourage an easy-fix kind of PR? Not really. So let's close this. Feel free to continue discussing of course.

ev-br avatar Jun 09 '23 16:06 ev-br