Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataArrayResample.interpolate and passing kwargs #8762

Closed
stackjohn opened this issue Feb 17, 2024 · 2 comments · Fixed by #9079
Closed

DataArrayResample.interpolate and passing kwargs #8762

stackjohn opened this issue Feb 17, 2024 · 2 comments · Fixed by #9079
Labels
needs triage Issue that has not been reviewed by xarray team member

Comments

@stackjohn
Copy link

What is your issue?

The docs here state that it is possible to use polynomial interpolation when resampling.

If you try this though:

data_per_minute = ds.resample(
    valid_time="1min"
).interpolate("polynomial")

You get the following:

ValueError: order is required when method=polynomial

The docs here state that you need to pass in order, how is this possible?

If you then try:

data_per_minute = ds.resample(
    valid_time="1min"
).interpolate(kind="polynomial", order=3)

You get:

TypeError: Resample.interpolate() got an unexpected keyword argument 'order'

How can the order be specified? Thank you in advance.

@stackjohn stackjohn added the needs triage Issue that has not been reviewed by xarray team member label Feb 17, 2024
@etienneschalk
Copy link
Contributor

import xarray as xr
import pandas as pd 
import numpy as np

Reproduce the bug

Example taken from DataArray.resample, adapted to name the dimension valid_time

xda = xr.DataArray(
    np.linspace(0, 11, num=12),
    coords={"valid_time":
        pd.date_range(
            "1999-12-15",
            periods=12,
            freq=pd.DateOffset(months=1),
        )
    },
)
print(xda)
try:
    data_per_day = xda.resample(valid_time="1D").interpolate("polynomial")
except ValueError as err:
    assert str(err) == "order is required when method=polynomial"
try:
    data_per_day = xda.resample(valid_time="1D").interpolate(kind="polynomial", order=3)
except TypeError as err:
    assert (
        str(err) == "Resample.interpolate() got an unexpected keyword argument 'order'"
    )

Technical Analysis

First error message

"order is required when method=polynomial"

Source: https://github.com/pydata/xarray/blob/main/xarray/core/missing.py#L153

It seems that the method argument becomes the order one when method == 'polynomial'. It is unclear to me what the order argument is supposed to contain. Maybe @jhamman have an idea? This seems to have been introduced in #1640

Second error message:

"Resample.interpolate() got an unexpected keyword argument 'order'"

Check https://docs.xarray.dev/en/stable/generated/xarray.core.resample.DataArrayResample.interpolate.html

Source: https://github.com/pydata/xarray/blob/main/xarray/core/resample.py#L143-L171

We can see that the function signature only allows for a single kind keyword, hence the TypeError: Resample.interpolate() got an unexpected keyword argument 'order' is the direct consequence.

It uses interp1d. ⚠️ This function is declared as legacy, so maybe xarray should move away from its use?

xr.show_versions()

import warnings

warnings.filterwarnings("ignore")
xr.show_versions()

@stackjohn
Copy link
Author

@etienneschalk Thank you for adding much more detail 🙇

nkarasiak added a commit to nkarasiak/xarray that referenced this issue Jun 8, 2024
nkarasiak added a commit to nkarasiak/xarray that referenced this issue Jun 8, 2024
dcherian added a commit that referenced this issue Jun 11, 2024
* add order for polynomial, fixes #8762

* add bugfixes in whats_new.rst, fixes #8762

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix typo

* Update xarray/core/resample.py

setdefault "bounds_error", and not forcing it. Thanks @dcherian

Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>

* chore(test) : polynomial resample

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix(setdefault) : change dict to arg

* fix(test) : polynomial interpolate tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* chore(test) : add comment using interp1d for polynomial

* Update xarray/tests/test_groupby.py

* Update doc/whats-new.rst

* Update whats-new.rst

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
andersy005 pushed a commit that referenced this issue Jun 14, 2024
* add order for polynomial, fixes #8762

* add bugfixes in whats_new.rst, fixes #8762

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix typo

* Update xarray/core/resample.py

setdefault "bounds_error", and not forcing it. Thanks @dcherian

Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>

* chore(test) : polynomial resample

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix(setdefault) : change dict to arg

* fix(test) : polynomial interpolate tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* chore(test) : add comment using interp1d for polynomial

* Update xarray/tests/test_groupby.py

* Update doc/whats-new.rst

* Update whats-new.rst

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Deepak Cherian <dcherian@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage Issue that has not been reviewed by xarray team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants