issue_comments

Thanks @mark-boer ! Must be one of the largest first contributions...

+1 re merge + experimental warning; maybe we should do this more often. Cheers @dcherian

Merging. I've added an experimental warning to the docstrings and we can discuss the IndexVariable situation here: https://github.com/pydata/xarray/issues/3868

Thanks @mark-boer this is a major contribution for your first PR!

Agree!

We could add an "Experimental" label and then worry less about future changes

Extremely good points @mark-boer

I propose we merge and open an issue to decide what to do with IndexVariables.

In some instances extrapolating all coords, can lead to some unwanted behaviour. Would you suggest we only interpolate the indexes?

How would we handle unsorted indexes?

How would we extrapolate all the different kind of indexes, like the MultiIndex or CategoricalIndex?

I agree; I can't see easy solutions to these. If there are evenly spaced indexes (e.g. dates, grids), then it's easy to know what to do. But there are plenty of times it's difficult, if not intractable.

One option to merge something useful without resolving these questions is to return Variables only and label this method experimental. I think this is immediately useful for the cases where these difficult questions don't need to be answered.

Of course if there are good answers to the questions, then even better!

Hmm, I don't really see a solution. What do you suggest?

:) I think we need to extrapolate indexes by default. It seems like the most sensible option.

I do agree it can be confusing, but it is not unique in xarray. Dataset.shift only shifts data_vars, bfill and ffill only fill data_vars, etc.

I agree, I wouldn't have expected coords to be included given existing behavior. People can switch coords <> data_vars as needed, so there's an escape hatch

Edit: But it's more awkward for indexes than non-index coords. The index becomes less useful with non-unique values, and generally indexes don't have nulls. I'm not sure what the other options would be: to some extent it's the intersection of pad with xarray's data model.

This looks excellent @mark-boer , thank you!

I will try and have a proper look through today (but don't wait for me)

I pushed some minor changes.

I think this is ready to go in.

The big outstanding issue is what to do about dimension coordinates or indexes. Currently this PR treats all variables in coords different from those in data_vars. I think this is confusing.

I am thinking that we want to use linear extrapolation for IndexVariables by default and apply the same padding mode to all other variables. The reasoning being that IndexVariables with NaNs are hard to deal with and it's hard to fill them in: padded["x"] = padded.x.drop_vars("x").interpolate_na("x", fill_value="extrapolate")

It seems like we have some value mismatches on dask==1.2. @fujiisoup is this the error you found?

``` =================================== FAILURES =================================== _ TestVariableWithDask.testpad[xr_arg0-np_arg0-linear_ramp] __

self = <xarray.tests.test_variable.TestVariableWithDask object at 0x7fb69b629b00> mode = 'linear_ramp', xr_arg = {'x': (2, 1)}, np_arg = ((2, 1), (0, 0), (0, 0))

@pytest.mark.parametrize(
    "mode",
    [
        pytest.param("mean", marks=pytest.mark.xfail),
        pytest.param("median", marks=pytest.mark.xfail),
        pytest.param("reflect", marks=pytest.mark.xfail),
        "edge",
        "linear_ramp",
        "maximum",
        "minimum",
        "symmetric",
        "wrap",
    ],
)
@pytest.mark.parametrize(
    "xr_arg, np_arg",
    [
        [{"x": (2, 1)}, ((2, 1), (0, 0), (0, 0))],
        [{"y": (0, 3)}, ((0, 0), (0, 3), (0, 0))],
        [{"x": (3, 1), "z": (2, 0)}, ((3, 1), (0, 0), (2, 0))],
    ],
)
def test_pad(self, mode, xr_arg, np_arg):
    data = np.arange(4 * 3 * 2).reshape(4, 3, 2)
    v = self.cls(["x", "y", "z"], data)

    actual = v.pad(mode=mode, **xr_arg)
    expected = np.pad(data, np_arg, mode=mode,)

  assert_array_equal(actual, expected)

E AssertionError: E Arrays are not equal E
E (mismatch 2.3809523809523796%) E x: array([[[ 0, 0], E [ 0, 0], E [ 0, 0]],... E y: array([[[ 0, 0], E [ 0, 0], E [ 0, 0]],...

xarray/tests/test_variable.py:821: AssertionError _ TestVariableWithDask.testpad[xr_arg1-np_arg1-linear_ramp] __

self = <xarray.tests.test_variable.TestVariableWithDask object at 0x7fb69df9eb00> mode = 'linear_ramp', xr_arg = {'y': (0, 3)}, np_arg = ((0, 0), (0, 3), (0, 0))

@pytest.mark.parametrize(
    "mode",
    [
        pytest.param("mean", marks=pytest.mark.xfail),
        pytest.param("median", marks=pytest.mark.xfail),
        pytest.param("reflect", marks=pytest.mark.xfail),
        "edge",
        "linear_ramp",
        "maximum",
        "minimum",
        "symmetric",
        "wrap",
    ],
)
@pytest.mark.parametrize(
    "xr_arg, np_arg",
    [
        [{"x": (2, 1)}, ((2, 1), (0, 0), (0, 0))],
        [{"y": (0, 3)}, ((0, 0), (0, 3), (0, 0))],
        [{"x": (3, 1), "z": (2, 0)}, ((3, 1), (0, 0), (2, 0))],
    ],
)
def test_pad(self, mode, xr_arg, np_arg):
    data = np.arange(4 * 3 * 2).reshape(4, 3, 2)
    v = self.cls(["x", "y", "z"], data)

    actual = v.pad(mode=mode, **xr_arg)
    expected = np.pad(data, np_arg, mode=mode,)

  assert_array_equal(actual, expected)

E AssertionError: E Arrays are not equal E
E (mismatch 16.66666666666667%) E x: array([[[ 0, 1], E [ 2, 3], E [ 4, 5],... E y: array([[[ 0, 1], E [ 2, 3], E [ 4, 5],...

xarray/tests/test_variable.py:821: AssertionError _ TestVariableWithDask.testpad[xr_arg2-np_arg2-linear_ramp] __

self = <xarray.tests.test_variable.TestVariableWithDask object at 0x7fb69c609860> mode = 'linear_ramp', xr_arg = {'x': (3, 1), 'z': (2, 0)} np_arg = ((3, 1), (0, 0), (2, 0))

@pytest.mark.parametrize(
    "mode",
    [
        pytest.param("mean", marks=pytest.mark.xfail),
        pytest.param("median", marks=pytest.mark.xfail),
        pytest.param("reflect", marks=pytest.mark.xfail),
        "edge",
        "linear_ramp",
        "maximum",
        "minimum",
        "symmetric",
        "wrap",
    ],
)
@pytest.mark.parametrize(
    "xr_arg, np_arg",
    [
        [{"x": (2, 1)}, ((2, 1), (0, 0), (0, 0))],
        [{"y": (0, 3)}, ((0, 0), (0, 3), (0, 0))],
        [{"x": (3, 1), "z": (2, 0)}, ((3, 1), (0, 0), (2, 0))],
    ],
)
def test_pad(self, mode, xr_arg, np_arg):
    data = np.arange(4 * 3 * 2).reshape(4, 3, 2)
    v = self.cls(["x", "y", "z"], data)

    actual = v.pad(mode=mode, **xr_arg)
    expected = np.pad(data, np_arg, mode=mode,)

  assert_array_equal(actual, expected)

E AssertionError: E Arrays are not equal E
E (mismatch 5.208333333333329%) E x: array([[[ 0, 0, 0, 0], E [ 0, 0, 0, 0], E [ 0, 0, 0, 0]],... E y: array([[[ 0, 0, 0, 0], E [ 0, 0, 0, 0], E [ 0, 0, 0, 0]],... ```

Hi, @mark-boer. In #3587, I tried using dask's pad method but noticed a few bugs in older (but newer than 1.2) dask. For me, it would be very welcome if you add this method to dask_array_compat. Then, I would wait for merging #3587 until this PR is completed.

Thanks for your contribution :)