Fix/time series interpolation is wrong 21351

[cbpygit]

• closes Time Series Interpolation is wrong #21351
• Tests added and passed if fixing a bug or adding a new feature
• All code checks passed.
• Added type annotations to new arguments/methods/functions.
• Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

[jbrockmendel]

FAILED pandas/tests/resample/test_time_grouper.py::test_groupby_resample_interpolate - TypeError: '<' not supported between instances of 'int' and 'tuple'

Any idea what is causing this? Seems unlikely to be "expected is wrong" for this one (which i assume is the case for the other one)

[jbrockmendel]

in the opposite case where the difference is empty, i think a the sort_index and .loc below (which makes a copy) can be avoided

[cbpygit]

@jbrockmendel Can you please clarify? "in the opposite case where the difference is empty" --> then missing_data_points_index will be empty, so its length will be zero. In that case, the if-expression evaluates to False, so that the loc/sort is indeed not executed. Maybe I am overlooking something.

[jbrockmendel]

IIUC in the motivating case we have a series whose index has freq1 and a new freq2 that are not multiples of each other. Is what you're doing here similar to

freq3 = LCD(freq1, freq2)
result = series.resample(freq3).interpolate(...).asfreq(freq2)

And if so, would that be more efficient/clearer than what you're doing here? I guess it wouldn't work if the original index doesn't have a freq? Or might blow up if the LCD happens to be tiny?

[jbrockmendel]

is there a doctest this changes/fixes?

[cbpygit]

IIUC in the motivating case we have a series whose index has freq1 and a new freq2 that are not multiples of each other. Is what you're doing here similar to

freq3 = LCD(freq1, freq2)
result = series.resample(freq3).interpolate(...).asfreq(freq2)

And if so, would that be more efficient/clearer than what you're doing here? I guess it wouldn't work if the original index doesn't have a freq? Or might blow up if the LCD happens to be tiny?

@jbrockmendel I don't think this is a "pathological" case, but in general, this type of resampling should be agnostic to what sort of index it is.

It is helpful to think of the input data as a "measurement" of a function $f(x)$ at some points $x_i$, and the resampling/interpolation as asking the question about function values at a "higher number" of points $x_j$. The code identifies this correctly as upsampling. The interpolator is a "piecewise assumed function" $g(x)$ that (hopefully) describes how the data behaves between adjacent points $x_i, x_{i+1}$. It is irrelevant whether the original sampling followed a precise frequency. Instead, it is important that the interpolator can take into account as many points in the vicinity of the new points $x_j$ to evaluate $g$. (In the simple linear case 2 adjacent points are enough, but for more complex interpolation more anchor points may be used.)

The problem with the previous implementation is that those anchor points were, more or less randomly, excluded from the interpolation process. All we need to do is make sure none of the original data points is missing because this would mean throwing away valid information. This is achieved by

result = concat(
    [result, obj.loc[missing_data_points_index]]
)

The remaining code is just gymnastics around this. I can help make this more clear in the code or here, but I don't think it can be simplified further.

[cbpygit]

FAILED pandas/tests/resample/test_time_grouper.py::test_groupby_resample_interpolate - TypeError: '<' not supported between instances of 'int' and 'tuple'

Any idea what is causing this? Seems unlikely to be "expected is wrong" for this one (which i assume is the case for the other one)

@jbrockmendel I spent quite a while looking at this now. The problem here is caused by interpolation applied to a GroupBy object. I was not able to solve how I would access/recreate the MultiIndex that the original data frame would have in Resampler.interpolate if it was not resampled 😞. The resampling happens somewhere in self._upsample. So there is a lack of information here, in a sense. I think I would need help to make it work for the groupby case, and it would feel unnatural to support it for non-grouped, but not grouped operations.

[cbpygit]

@jbrockmendel I took another look and added an implementation for multi-indexes (resulting from groupby). This fixes test_groupby_resample_interpolate, all tests in resample are green for me now locally.

[cbpygit]

And regarding pandas.tests.series.methods.test_interpolate.TestSeriesInterpolateData.test_interpolate, which seems to be the test that crashes in most of the other test suites: I don't see how this test was ever passing 🤷‍♂️. The input datetime_series has an index created using date_range with freq="B". So this index is not even spaced, as it has only business days. A linear interpolation will therefore never result in np.arange(len(datetime_series), dtype=float).

[MarcoGorelli]

thanks for your PR

there's still some failing tests:

FAILED pandas/tests/series/methods/test_interpolate.py::TestSeriesInterpolateData::test_interpolate - AssertionError: Series are different

Series values are different (13.33333 %)
[index]: [2000-01-03T00:00:00.000000000, 2000-01-04T00:00:00.000000000, 2000-01-05T00:00:00.000000000, 2000-01-06T00:00:00.000000000, 2000-01-07T00:00:00.000000000, 2000-01-10T00:00:00.000000000, 2000-01-11T00:00:00.000000000, 2000-01-12T00:00:00.000000000, 2000-01-13T00:00:00.000000000, 2000-01-14T00:00:00.000000000, 2000-01-17T00:00:00.000000000, 2000-01-18T00:00:00.000000000, 2000-01-19T00:00:00.000000000, 2000-01-20T00:00:00.000000000, 2000-01-21T00:00:00.000000000, 2000-01-24T00:00:00.000000000, 2000-01-25T00:00:00.000000000, 2000-01-26T00:00:00.000000000, 2000-01-27T00:00:00.000000000, 2000-01-28T00:00:00.000000000, 2000-01-31T00:00:00.000000000, 2000-02-01T00:00:00.000000000, 2000-02-02T00:00:00.000000000, 2000-02-03T00:00:00.000000000, 2000-02-04T00:00:00.000000000, 2000-02-07T00:00:00.000000000, 2000-02-08T00:00:00.000000000, 2000-02-09T00:00:00.000000000, 2000-02-10T00:00:00.000000000, 2000-02-11T00:00:00.000000000]
[left]:  [0.0, 1.0, 2.0, 3.0, 4.0, 5.8, 6.4, 7.0, 7.6, 8.2, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0, 17.0, 18.0, 19.0, 20.0, 21.0, 22.0, 23.0, 24.0, 25.0, 26.0, 27.0, 28.0, 29.0]
[right]: [0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0, 16.0, 17.0, 18.0, 19.0, 20.0, 21.0, 22.0, 23.0, 24.0, 25.0, 26.0, 27.0, 28.0, 29.0]
At positional index 5, first diff: 5.8 != 5.0
FAILED pandas/tests/series/methods/test_interpolate.py::TestSeriesInterpolateData::test_nan_irregular_index - AssertionError: Series are different

If these were wrong to begin with, could you please updated them so they have the correct values so that they pass? That would need doing as part of this PR

CI would need to be green for this to be mergeable (furthermore, people are much more likely to review a PR which is green)

[cbpygit]

All green now @MarcoGorelli @jbrockmendel

[MarcoGorelli]

thanks - aiming to get to this this month

[cbpygit]

I don't think the failing test cases are related to my code @MarcoGorelli @mroeschke , what do I do in this case? It occurred after syncing with main.

[MarcoGorelli]

Looks good to me, thanks @cbpygit for having stuck with this one!

Leaving open a bit in case there's further comments

[mroeschke]

Could you add a whatsnew note in v3.0.0.rst under the Other section of the Bug Fix group? Thanks for sticking with this almost there!

[mroeschke]

Thanks @cbpygit