Support vectorized interpolation with more scipy interpolators

[hollymandel]

• ClosesCannot use documented interp() methods due to "vectorizeable_only" check and kwargs name clash #9049
• Adds support for vectorized (broadcast) 1d interpolation using SciPy interpolants, including "akima" and "makima"
• Does not add support for deprecated SplineInterpolator (but upholds preexisting functionality). I wanted to add a deprecation warning but this caused tests to fail so I left it commented.
• Akima and makima cases added to test_interp.py::test_interpolate_vectorize

[max-sixty]

Something has gone kinda wrong with git here! Maybe try squashing to a single commit?

[hollymandel]

Something has gone kinda wrong with git here! Maybe try squashing to a single commit?

yes, done, thanks!

[max-sixty]

Nice, thanks!

(I don't know this code well so will defer to others on the review, hope that's OK...)

[hollymandel]

Sounds good! I actually wanted it to be a "draft PR" as I'm still double-checking it, though perhaps submitting it this way is confusing on the maintainers' end and I should rescind until it's ready.

[keewis]

though perhaps submitting it this way is confusing on the maintainers' end and I should rescind until it's ready

we're actually happy to have PRs developed like this, it allows us to guide people more easily in case there's any issues. This includes advising in case the direction is seriously wrong (which I don't think it is in this case, but I also don't know this area too well)

Just tell us when you think this is ready or if you need any help.

[dcherian]

nice cleanup!

[dcherian]

Suggested change

	ifnothas_scipy:
	pytest.skip("scipy is not installed.")

driveby cleanup. The requires_scipydecorator will skip the test if scipy isn't available. Import it from xarray.tests if it's not present.

[dcherian]

1. The addition of .data is significant. Where did the types change? We seem to have moved from np.ndarray to Variable?
2. Is there a reason you got rid of assume_sorted=True?

[hollymandel]

1. This doesn't seem to have been necessary, changing back

2. The interpolators I have added (Scipy.Interpolate.Akima1DInterpolator, etc...) require sorted axes but don't take assume_sorted as an argument. Only Scipy.Interpolate.Interp1d takes this argument. Since we are always calling it with True, I'm just letting the default for ScipyInterpolator init impose this.

There is maybe a philosophical conflict here and in previous comments about kwarg handling for the different interpolators. The preexisting script (missing.py) has special classes (NumpyInterpolator, ScipyInterpolator...) which basically just do this handling. I didn't add any new classes for the new interpolators and rather tried to do everything with kwargs determined by _get_interpolator.

[dcherian]

Re (1): If you're comfortable with typing, some type hints would go a long way towards improving this module.

Re (2): sounds good

[hollymandel]

(1) Yeah can do! Prefer a separate commit though (?)

[dcherian]

we squash merge so it doesn't matter :) unless you meant separate PR, in which case that would be fine too.

[dcherian]

Nice progress! I pushed a commit that should handle the min scipy version failure with "makima".

Please add a note to whats-new.rst also.

I wanted to add a deprecation warning

Yes, we error on unexpected warnings. To add this, catch the warning using

withpytest.warns(DeprecationWarning):
...

though perhaps submitting it this way is confusing on the maintainers' end and I should rescind until it's ready

It also let us handle the annoying pieces like min scipy versions ;). We have established patterns to handle such things, but its hard for a new contributor to find them at first glance

[dcherian]

(fyi we squash on merge, so you needn't squash every change)

[dcherian]

Very nice work! Thanks for taking this on, we've needed someone to do this for a while now :)

One last request, please add a note to whats-new.rst

[max-sixty]

Thanks @hollymandel !