interpolate_na: Add max_gap support.

[dcherian]

• Closes Improving interpolate_na()'s limit argument #2392
• Tests added
• Passes black . && mypy . && flake8
• Fully documented, including whats-new.rst for all changes and api.rst for new API

@dnowacki-usgs : can you look this over and test it out if you have time? feel free to push any changes to this branch :)

[max-sixty]

Awesome, looks good!

I had a cursory check through the algo; a couple more scenarios tested would be good, in particular gaps in the middle of the values

Any thoughts on maxgap? limit_gap? Neither clicks that well!

[dcherian]

Thanks @max-sixty I've updated the tests.

Any thoughts on maxgap? limit_gap? Neither clicks that well!

I pulled maxgap from this stalled Pandas PR: pandas-dev/pandas#25141
OK it looks like that PR is alive again, so maybe it's good to keep the same kwarg?

[max-sixty]

OK it looks like that PR is alive again, so maybe it's good to keep the same kwarg?

Ah, agree we should align. I'm really not keen on that name but yes on balance; unless they're open to changing

[max-sixty]

As per the pandas issue, sounds like max_gap is consensus

[dcherian]

👍

[stefraynaud]

Nice feature.
How about adding the support max gaps expressed in physical units, since coordinates may be irregular?

[dcherian]

Thanks @stefraynaud . I'm having trouble figuring out defining the length of a gap in the irregular coordinate case.

e.g.

da4 = xr.DataArray([np.nan, np.nan, np.nan, 1, np.nan, np.nan, 4, np.nan, np.nan], 
                   dims=["y"], coords={"y": [0, 2, 5, 6, 7, 8, 10, 12, 14]})

<xarray.DataArray (y: 9)>
array([nan, nan, nan,  1., nan, nan,  4., nan, nan])
Coordinates:
  * y        (y) int64 0 2 5 6 7 8 10 12 14

What is the length of these three gaps given that xarray doesn't have any understanding of grids?

[max-sixty]

I think using locations rather than counts would be great, but harder and doesn't have to be part of this PR.

In the example above, it looks like 1 is aligned with 6 and 4 with 10, so the gap in locations along the y dimension would be 4?

[dcherian]

OK added test and now raises error for irregularly spaced coordinates. I agree that this should be good for now.

[max-sixty]

Forgive me if I'm being slow—is the max_gap measuring the locations (i.e. on the index), or the number of values? The example below seems to be counting two values, rather than measuring the space between the locations.

If that's right, why does it matter than the coordinates are irregular (or even non-monotonic)?

[stefraynaud]

Thanks @stefraynaud . I'm having trouble figuring out defining the length of a gap in the irregular coordinate case.

e.g.

da4 = xr.DataArray([np.nan, np.nan, np.nan, 1, np.nan, np.nan, 4, np.nan, np.nan], 
                   dims=["y"], coords={"y": [0, 2, 5, 6, 7, 8, 10, 12, 14]})

<xarray.DataArray (y: 9)>
array([nan, nan, nan,  1., nan, nan,  4., nan, nan])
Coordinates:
  * y        (y) int64 0 2 5 6 7 8 10 12 14

What is the length of these three gaps given that xarray doesn't have any understanding of grids?

@dcherian In your example, as said @max-sixty, the middle gap has a length of 10-6=4. The length gaps at the edges cannot be computed but it doesn't matter, and the algo should work as when simply counting the nans.

I'll have a look the code, maybe for a new PR after this one.

[dcherian]

The thing I find weird is that for

<xarray.DataArray (y: 9)>
array([nan, nan, nan,  1., nan, nan,  4., nan, nan])
Coordinates:
  * y        (y) int64 0 1 2 3 4 5 6 7 8

the center gap's length = 7-4 = 3 which is the number of NaNs + 1. But maybe this is OK.

1. We should check what that pandas PR does and align with that
2. interp calls scipy.interpolate.interp which does do extrapolation, so we should figure out a sensible solution for the edges (extrapolating coordinates using the first and last spacing seems reasonable to me).

@stefraynaud I don't have time to work on this now. Please feel free to modify this and open a new PR. You could try to push to this branch but I'm not sure it will work.

[max-sixty]

IIUC, and please correct me if I'm wrong, the pandas version counts points rather than the distance between locations. Ideally we'd be able to do both, but even if we can only have one working correctly, that would be v good

[dcherian]

yes, I think you are right. I was thinking that it would be nice to have the number-of-nan-points and gap-length metrics converge for uniformly spaced coordinates but I don't think that's possible in any sensible way.

[dcherian]

Thanks @dnowacki-usgs that's a nice test. I think the right fix is to make index a Variable so that we get automatic broadcasting.

Things left to do (or at least add xfail tests + errors):

• support for cftime indexes and offsets
• what to do when use_coordinate=False
• what to do with unlabeled dimensions
• document convention for gap length
• add examples to docs

[dcherian]

Done for now. Ready for final review / testing.

[dcherian]

This could use another round of testing / review

(cc @dnowacki-usgs @stefraynaud @max-sixty )

[dnowacki-usgs]

Thanks for all your work @dcherian! Did a quick test with some real-world timeseries data I've been wanting to use with max_gap and it looks good to me. I will definitely be using this in the future! 👍

[dcherian]

I'm going to merge this. Happy to make any other changes.

[max-sixty]

Great, thanks @dcherian !