pydata / pydata/xarray

DataTree consolidation to use general AttrAccessMixin class (instead of TreeAttrAccessMixin)

Open
#9,068 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

topic-DataTree topic-internals
Dominant language
Python
Stars
4.2k
Forks
1.4k
Avg merge
2d 15h
Merged PRs (30d)
14

Description

What is your issue?

In #9011 code from datatree was merged into the main xarray/core subdirectory. This includes the TreeAttrAccessMixin class, which is inherited by DataTree. There is also a more general AttrAccessMixin class, which would be preferable to use instead.

The reason that AttrAccessMixin is not just used for DataTree is that AttrAccessMixin.__init_subclass__ has a check to see if there is a __dict__ attribute on the class. If there is, an error is thrown, saying that __slots__ should be used instead. DataTree (and TreeNode and NamedNode) do define __slots__, but there are some attributes that are also declared dynamically (e.g., DataTree.children and DataTree.parent, which are first defined in DataTree.__init__). Because of these dynamic declarations, DataTree has both __slots__ and __dict__ and therefore fails the check in AttrAccessMixin.__init_subcass__.

This issue aims to update DataTree (and potentially NamedNode and TreeNode, if needed), such that DataTree can pass the check in AttrAccessMixin.__init_subclass__ and the additional TreeAttrAccessMixin class can be removed.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in xarray/core by reading AttrAccessMixin, TreeAttrAccessMixin, and the DataTree, TreeNode, and NamedNode definitions. Check how slots and dynamic attributes create dict, then verify that DataTree passes AttrAccessMixin.init_subclass; done means TreeAttrAccessMixin can be removed and the relevant classes use the general mixin.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
data
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.