Skip to content

some private imports broken on main #6741

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

Closed
dcherian opened this issue Jun 30, 2022 · 6 comments · Fixed by #6750
Closed

some private imports broken on main #6741

dcherian opened this issue Jun 30, 2022 · 6 comments · Fixed by #6750
Labels

Comments

@dcherian
Copy link
Contributor

dcherian commented Jun 30, 2022

What happened?

Seen over in cf_xarray

Using xr.core.resample.Resample worked prior to #6702.

Now we need to use from xarray.core.resample import Resample

I don't know if this is something that needs to be fixed or only worked coincidentally earlier. But I thought it was worth discussing prior to release.

Thanks to @aulemahal for spotting

@dcherian dcherian added the bug label Jun 30, 2022
@aulemahal
Copy link
Contributor

To add details, I suspect this commit as the culprit:
headtr1ck@cd2f2b2

It removes a direct import of the core.resample submodule in core.dataarray and moves it to the "Type checking" imports. Thus, importing xarray (import xarray as xr), does not trigger an explicit import of resample anymore.

@headtr1ck
Copy link
Collaborator

It was a semi-incidential change.
These modules are now only imported when required (i.e. calling DataArray.rolling ), I didn't think anyone would actually use these classes directly.

The modules can either be imported at DataArray/set module level directly again, or we could add a breaking change note.

@headtr1ck
Copy link
Collaborator

On the plus side, with the current implementation we probably save a few ms import time xD.

@Illviljan
Copy link
Contributor

We do at least warn users that using anything from .core can change at any time,
https://xarray.pydata.org/en/stable/getting-started-guide/faq.html?highlight=private#what-parts-of-xarray-are-considered-public-api

@Illviljan
Copy link
Contributor

Illviljan commented Jun 30, 2022

Another example is that asfreq has moved downstream a little and is no longer available on Resample either, can probably break something as well.

The docstring also warns to use this class directly since it behaves as an incomplete base class.

"""An object that extends the `GroupBy` object with additional logic
for handling specialized re-sampling operations.
You should create a `Resample` object by using the `DataArray.resample` or
`Dataset.resample` methods. The dimension along re-sampling

@headtr1ck
Copy link
Collaborator

Another example is that asfreq has moved downstream a little and is no longer available on Resample either, can probably break something as well.

It was moved because it actually didn't work in the baseclass ;)
So I don't think anyone has ever used that.

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

Successfully merging a pull request may close this issue.

4 participants