-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Static typing for popular accessors #11087
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
base: main
Are you sure you want to change the base?
Changes from all commits
aa4c5b1
05aeef0
22314ea
3eacdd5
477f1be
e836ec6
bb222e6
4c83113
4c73974
40a2f2c
ce31134
618c1c5
1fed2ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,53 @@ | ||||||
| """ | ||||||
| External accessor support for xarray. | ||||||
|
|
||||||
| This module provides mixin classes with typed properties for external accessor | ||||||
| packages, enabling full IDE support (autocompletion, parameter hints, docstrings) | ||||||
| for packages like hvplot, cf-xarray, pint-xarray, rioxarray, and xarray-plotly. | ||||||
|
|
||||||
| Properties are defined statically for IDE support | ||||||
| """ | ||||||
|
|
||||||
| from __future__ import annotations | ||||||
|
|
||||||
| from typing import TYPE_CHECKING, ClassVar | ||||||
|
|
||||||
| if TYPE_CHECKING: | ||||||
| from cf_xarray.accessor import CFAccessor | ||||||
| from hvplot.xarray import hvPlotAccessor | ||||||
| from pint_xarray import PintDataArrayAccessor, PintDatasetAccessor | ||||||
| from rioxarray import RasterArray, RasterDataset | ||||||
| from xarray_plotly import DataArrayPlotlyAccessor, DatasetPlotlyAccessor | ||||||
|
Comment on lines
+15
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So do all of these unrelated packages now have to be installed in our CI (in the same environment too)? Plotly at least is new.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually they don't need to be installed at all since they are only imported within the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jsignell Pretty elegant solution with the MixIn. I like it!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does the IDE completion/hints still work even if not all of these are installed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DocOtak Yes they do. But they are deprioritized (at least in my IDE), saying they are not at the top of the list Here with only
But after I type in Completion after the accessor name is only available for installed accessors:
One more thing: The existing This means with conflicting var_names and accessor names, the code behaviour is backwards compatible. Only conflict in type hints, not at Runtime.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this "break" mypy for people only using xarray and only having installed a few or none accessors? |
||||||
|
|
||||||
|
|
||||||
| class DataArrayExternalAccessorMixin: | ||||||
| """Mixin providing typed external accessor properties for DataArray.""" | ||||||
|
|
||||||
| __slots__ = () | ||||||
|
|
||||||
| hvplot: ClassVar[type[hvPlotAccessor]] | ||||||
| cf: ClassVar[type[CFAccessor]] | ||||||
| pint: ClassVar[type[PintDataArrayAccessor]] | ||||||
| rio: ClassVar[type[RasterArray]] | ||||||
| plotly: ClassVar[type[DataArrayPlotlyAccessor]] | ||||||
|
|
||||||
|
|
||||||
| class DatasetExternalAccessorMixin: | ||||||
| """Mixin providing typed external accessor properties for Dataset.""" | ||||||
|
|
||||||
| __slots__ = () | ||||||
|
|
||||||
| hvplot: ClassVar[type[hvPlotAccessor]] | ||||||
| cf: ClassVar[type[CFAccessor]] | ||||||
| pint: ClassVar[type[PintDatasetAccessor]] | ||||||
| rio: ClassVar[type[RasterDataset]] | ||||||
| plotly: ClassVar[type[DatasetPlotlyAccessor]] | ||||||
|
|
||||||
|
|
||||||
| class DataTreeExternalAccessorMixin: | ||||||
| """Mixin providing typed external accessor properties for DataTree.""" | ||||||
|
|
||||||
| __slots__ = () | ||||||
|
|
||||||
| hvplot: ClassVar[type[hvPlotAccessor]] | ||||||
| cf: ClassVar[type[CFAccessor]] | ||||||






There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to fix "Your Name"?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. But it's not my PR anymore, so I'm not sherif I can. And I don't have a laptop at hand for the next few days.