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.

Sign up for GitHub

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

include accessors in __dir__ #9985

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+36 −6
Open

include accessors in __dir__ #9985

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5314f02
add `_accessors` as a place to register accessor names
keewis Jan 25, 2025
4dc0527
include the accessor in the new set upon registry
keewis Jan 25, 2025
bc5a25b
include the accessor names in `__dir__`
keewis Jan 25, 2025
9433b64
actually register on `DataTree`
keewis Jan 25, 2025
2b7d0c8
check that the registered accessors are included in `__dir__`
keewis Jan 25, 2025
050d834
fix the wrong type hint
keewis Jan 25, 2025
9862105
changelog
keewis Jan 25, 2025
580fd3f
mark `_accessor` as a class variable
keewis Jan 26, 2025
bdc7984
access `_accessors` as a class variable
keewis Jan 26, 2025
92d9f89
Merge branch 'main' into accessors-dir
dcherian Jan 27, 2025
336eb26
use a mixin to provide the extension discoverability
keewis Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions doc/whats-new.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Bug fixes
By `Kai Mühlbauer <https://github.com/kmuehlbauer>`_.
- Fix weighted ``polyfit`` for arrays with more than two dimensions (:issue:`9972`, :pull:`9974`).
By `Mattia Almansi <https://github.com/malmans2>`_.
- Include accessors in ``__dir__`` (:pull:`9985`).
By `Justus Magin <https://github.com/keewis>`_.

Documentation
~~~~~~~~~~~~~
Expand Down
2 changes: 1 addition & 1 deletion xarray/core/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ def __dir__(self) -> list[str]:
for item in source
if isinstance(item, str)
}
return sorted(set(dir(type(self))) | extra_attrs)
return sorted(set(dir(type(self))) | self._accessors | extra_attrs)

def _ipython_key_completions_(self) -> list[str]:
"""Provide method for the key-autocompletions in IPython.
Expand Down
1 change: 1 addition & 0 deletions xarray/core/dataarray.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ class DataArray(
_indexes: dict[Hashable, Index]
_name: Hashable | None
_variable: Variable
_accessors: set[str] = set()

__slots__ = (
"__weakref__",
Expand Down
1 change: 1 addition & 0 deletions xarray/core/dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ class Dataset(
_close: Callable[[], None] | None
_indexes: dict[Hashable, Index]
_variables: dict[Hashable, Variable]
_accessors: set[str] = set()

__slots__ = (
"__weakref__",
Expand Down
1 change: 1 addition & 0 deletions xarray/core/datatree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ class DataTree(
_attrs: dict[Hashable, Any] | None
_encoding: dict[Hashable, Any] | None
_close: Callable[[], None] | None
_accessors: set[str] = set()

__slots__ = (
"_attrs",
Expand Down
1 change: 1 addition & 0 deletions xarray/core/extensions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def decorator(accessor):
stacklevel=2,
)
setattr(cls, name, _CachedAccessor(name, accessor))
cls._accessors.add(name)
return accessor

return decorator
Expand Down
4 changes: 4 additions & 0 deletions xarray/tests/test_extensions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def __init__(self, xarray_obj):
def foo(self):
return "bar"

assert "demo" in dir(xr.DataTree)
assert "demo" in dir(xr.Dataset)
assert "demo" in dir(xr.DataArray)
Comment on lines +37 to +39
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this PR also allow completion of the actual accessor methods, or just the accessor namespace? If so then we should test that too.

Copy link
Collaborator Author

@keewis keewis Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does, but but how would you test that? This is functionality of the auto-completer...

Edit: to be clear, I think auto-completion of the namespace was already working before this PR

Copy link
Collaborator Author

@keewis keewis Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, I just tried dir(ds) on main, and it included the accessor... so it might be that this PR is not necessary? Can anyone confirm that? Or did I manage to confuse myself?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does, but but how would you test that?

Surely just

ds = xr.Dataset()
assert "foo" in dir(ds.demo)

?

I just tried dir(ds) on main, and it included the accessor... so it might be that this PR is not necessary? Can anyone confirm that? Or did I manage to confuse myself?

I just tried seeing if xr.Dataset().d would suggest xr.Dataset().dt (because that's a built-in accessor) and for me it does not.

Copy link
Collaborator Author

@keewis keewis Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dt and str are only on DataArray

Edit: And assert "foo" in dir(ds.demo) should be the same as assert "foo" in dir(DemoAccessor), no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of you using a different IDE that does show the expected code-completion suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @keewis is right, most IDEs nowadays support auto-completion via LSP servers based on (somewhat strict) static code analysers such as pyright / pylance. So I'm afraid there's nothing much we can do to support auto-completion for dynamic attributes.

IDE configs using jedi under the hood may support it, although IIUC jedi operates under different modes (Interpreter vs. Script) so I'm not sure auto-completion for dynamic attributes will always work (probably only in the Interpreter mode?).

within the IDE there's still something that has to be evaluated with regard to decorators in order for the final definition of the things that are decorated to be available to the IDE.

Decorators are just syntax sugar for functions applied on other functions or classes, so if the decorator function is properly typed (i.e., its return type is clear or can be statically inferred) any static code analyser will be able to provide auto-completions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to enable this for mypy here: #7117
But failed because the documentation on mypy plugins is non existent.

Maybe something similar is possible for pyright, pylance?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the insight @benbovy. Sadly, for me at least, this seems to confirm one of the reasons that I'm not a fan of dynamically registered accessors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@max-sixty, if I remember correctly, you're also using VSCode? Do you have any additional insight here?

Sorry, just saw this.

if I understand correctly, VSCode builds its code completion on type stubs and type inference (through pylance), which means that dynamic attributes like accessors may not be possible to autocomplete.

This is also my understanding — a kernel will give better info than an IDE because it's doing something like calling dir(foo) rather than statically analyzing the code...


dt: xr.DataTree = xr.DataTree()
assert dt.demo.foo == "bar"

Expand Down
Loading