pydata / xarray

N-D labeled arrays and datasets in Python
https://xarray.dev
Apache License 2.0
3.64k stars 1.09k forks source link

Passing extra keyword arguments to `curvefit` throws an exception. #6891

Open zoj613 opened 2 years ago

zoj613 commented 2 years ago

What happened?

Just like the title says, passing an extra keyword argument corresponding to scipy's curve_fit throws an exception. The documentation has a parameter section that says:

*kwargs (optional) – Additional keyword arguments to passed to scipy curve_fit

So if one specifies a method="trf" keyword argument to the .curvefit method, you get an error: TypeError: curvefit() got an unexpected keyword argument 'method'

The only way it works as expected is if I pass the keyword arguments as dictionary elements via a kwargs argument like so: kwargs={"method": "trf"}. This behaviour contradicts what is mentioned in the docstring.

What did you expect to happen?

No error thrown

Minimal Complete Verifiable Example

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

da = xr.DataArray(

    np.random.rand(4, 3),

    [

        ("time", pd.date_range("2000-01-01", periods=4)),

        ("space", ["IA", "IL", "IN"]),

    ],

)
da.curvefit(coords=["time"], func=lambda x, params: x, method="trf")

MVCE confirmation

Relevant log output

TypeError: curvefit() got an unexpected keyword argument 'method'

Anything else we need to know?

No response

Environment

```shell commit: None python: 3.8.12 | packaged by conda-forge | (default, Jan 30 2022, 23:42:07) [GCC 9.4.0] python-bits: 64 OS: Linux OS-release: 5.4.0-1061-aws machine: x86_64 processor: x86_64 byteorder: little LC_ALL: None LANG: C.UTF-8 LOCALE: ('en_US', 'UTF-8') libhdf5: 1.12.1 libnetcdf: 4.8.1 xarray: 2022.3.0 pandas: 1.4.3 numpy: 1.22.0 scipy: 1.6.2 netCDF4: 1.6.0 pydap: None h5netcdf: 0.15.0 h5py: 3.7.0 Nio: None zarr: 2.10.3 cftime: 1.6.1 nc_time_axis: None PseudoNetCDF: None rasterio: None cfgrib: 0.9.10.1 iris: None bottleneck: None dask: 2022.03.0 distributed: 2022.3.0 matplotlib: None cartopy: None seaborn: None numbagg: None fsspec: 2022.01.0 cupy: None pint: None sparse: 0.13.0 setuptools: 63.1.0 pip: 22.2.2 conda: None pytest: 6.2.5 IPython: 8.4.0 sphinx: None ```
TomNicholas commented 2 years ago

Hi @zoj613 , thanks for the bug report. This should be a very simple fix, and your MVCE would make a great test for the behaviour. Would you be interested in contributing to xarray by submitting a PR with a fix? (No worries if you would rather we fix it, but if you are then we are always happy to welcome new contributors :grinning: )

husainridwan commented 1 year ago

I think this depends on the version of xarray you’re using. Newer versions from at least 0.16.0 and later now support your MVCE. If you’re still using an older version, you’ll have to pass it as dictionary elements as you said. @zoj613 @TomNicholas, you can check this out.

zoj613 commented 1 year ago

@alrho007 I still get this error using version 2022.9.0:

In [1]: import pandas as pd                                                                                                                                                                                                                                                                                            [5/182]
   ...: import xarray as xr
   ...: import numpy as np
   ...:
   ...: da = xr.DataArray(
   ...:
   ...:     np.random.rand(4, 3),
   ...:
   ...:     [
   ...:
   ...:         ("time", pd.date_range("2000-01-01", periods=4)),
   ...:
   ...:         ("space", ["IA", "IL", "IN"]),
   ...:
   ...:     ],
   ...:
   ...: )
   ...: da.curvefit(coords=["time"], func=lambda x, params: x, method="trf")
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[1], line 18
      3 import numpy as np
      5 da = xr.DataArray(
      6
      7     np.random.rand(4, 3),
   (...)
     16
     17 )
---> 18 da.curvefit(coords=["time"], func=lambda x, params: x, method="trf")

TypeError: curvefit() got an unexpected keyword argument 'method'

In [2]: xr.__version__
Out[2]: '2022.9.0'
kmuehlbauer commented 1 year ago

@zoj613 Could you try to provide kwargs=kwargs? Where kwargs is a dict with method-key inside.

kmuehlbauer commented 1 year ago

Nevermind, I should read the complete issue, not only the last comment.

I think this is just a problem with the doc-string then.

husainridwan commented 1 year ago

@TomNicholas I would like to work on this. Where do I start from?

Amisha2778 commented 1 year ago

Hey! I'm an Outreachy intern, I would like to work on this issue.

Amisha2778 commented 1 year ago

@TomNicholas Could you please help me out. Can I work on this issue?

headtr1ck commented 1 year ago

I think we should continue the discussion in https://github.com/pydata/xarray/issues/7130 since it is not clear which function signature (kwargs vs **kwargs) is preferred.

headtr1ck commented 1 year ago

Actually, as @kmuehlbauer pointed out, we can address this for now by updating the docstring of the function. It should get clear the the extra kwargs are passed as a dict and not as **kwargs.

Amisha2778 commented 1 year ago

@headtr1ck

Screenshot 2023-03-27 at 12 49 48 AM Screenshot 2023-03-27 at 12 49 30 AM

I tried by making some changes. There was only one function that I could find with the issue mentioned. But, I am getting some failed test cases.

headtr1ck commented 1 year ago

As said above, you don't have to change code to make it work. The intended way of using is da.curvefit(...., kwargs={"method": "trf"}).

You can submit a PR which changes **kwargs to kwargs in the docstring (both, Dataset.curvefit and DataArray.curvefit) and probably add some more info there.

Amisha2778 commented 1 year ago

Got it, thank you!