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

Define slots explicitly #132

Closed
bdestombe opened this issue Jul 29, 2020 · 5 comments
Closed

Define slots explicitly #132

bdestombe opened this issue Jul 29, 2020 · 5 comments
Labels
wontfix This will not be worked on
Milestone

Comments

@bdestombe
Copy link
Collaborator

Describe the bug
Upon importing the library:

/srv/conda/envs/notebook/lib/python3.7/typing.py:845: FutureWarning: xarray subclass DataStore should explicitly define __slots__
  super().__init_subclass__(*args, **kwargs)

To Reproduce
Import the dtscalibration library

Expected behavior
No warnings.

Additional context
Xarray, does not recommend subclassing (and thus using slots), but instead to put everything under an accessor:

ds.dts.single_ended_calibration()

For reasons listed here: http://xarray.pydata.org/en/stable/internals.html#extending-xarray
Mostly needed when combining multiple 'extensions' to xarray. But to my opinion, this requires a lot of work, breaks the current version, and would not change the functionality of the package.

@BSchilperoort BSchilperoort self-assigned this May 11, 2021
@BSchilperoort
Copy link
Collaborator

As for now I have decided to specifically silence this warning, as it just clutters the output and could confuse users. We might have to keep watching what xarray decides to do, but they seem to not have made any changes or decisions over the last year.

@BSchilperoort BSchilperoort added the wontfix This will not be worked on label May 12, 2021
@BSchilperoort BSchilperoort removed their assignment May 12, 2021
@BSchilperoort
Copy link
Collaborator

@bdestombe do we still need to tackle this issue? If our way of subclassing xarray is deprecated, it could break upon any new release of xarray. This might be a thing we should do before v3.0 😔

@bdestombe
Copy link
Collaborator Author

Hi Bart, this has been the case for several years now. Furthermore, I don't see any urgent deprecation warnings and therefor also don't think we have to tackle this now.

Instead, let's build an accessor in parallel to the current implementation.

@bdestombe bdestombe added this to the Accessor milestone Aug 1, 2023
@BSchilperoort
Copy link
Collaborator

I have been trying out accessors, and one problem is that it's difficult for code analyzers to determine what the type and available methods are of xarray/pandas extensions: microsoft/pylance-release#1112

In VS Code, the code analyzer does not see that regrid is a class under ds (Dataset), and that to_dataset is a valid function. No docstrings are displayed either:
image

@BSchilperoort BSchilperoort modified the milestones: Accessor, v3.0.0 Aug 11, 2023
@bdestombe bdestombe mentioned this issue Oct 21, 2023
3 tasks
@BSchilperoort
Copy link
Collaborator

Fixed by #204

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Development

No branches or pull requests

2 participants