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

Explicit xarray imports #343

Merged
merged 2 commits into from
Jun 29, 2022
Merged

Explicit xarray imports #343

merged 2 commits into from
Jun 29, 2022

Conversation

aulemahal
Copy link
Contributor

In accessor.py, a few xarray classes are used without their module being explicitly imported. It seems that the latest commits to xarray have modified some import behaviour and it broke cf-xarray. Specifically, it seems that:

import xarray as xr
from xarray import DataArray, Dataset
from xarray.core.arithmetic import SupportsArithmetic

is not sufficient to import xr.core.resample. See https://github.com/pangeo-data/xESMF/runs/7119669567?check_suite_focus=true

This PR fixes the problem simply by explicitly importing those "wrapped classes".

I'm not sure if this should be considered a breaking change for xarray. If yes, then the fix might be best done there, but I haven't found exactly the source of the bug. It was a lot simpler to do what I did here.

@dcherian
Copy link
Contributor

Yes this looks better in general.

I'm not sure if this should be considered a breaking change for xarray.

Me neither. @keewis do you have any thoughts here?

@dcherian dcherian merged commit b031b45 into main Jun 29, 2022
@dcherian dcherian deleted the fix-xr-import branch June 29, 2022 21:33
dcherian added a commit to dcherian/cf-xarray that referenced this pull request Jun 29, 2022
…with-flox

* 'main' of github.com:xarray-contrib/cf-xarray:
  Explicit xarray imports (xarray-contrib#343)
@keewis
Copy link
Contributor

keewis commented Jun 30, 2022

Not sure. The aim of the PR that introduced this change (pydata/xarray#6702) was to improve the typing, so this was probably a unintended side-effect. We should probably open a issue to see what the others think? We can import .core.{resample,weighted,groupby,rolling} in xarray.__init__ if it was indeed unintentional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants