Skip to content

Disallow passing a DataArray as data into the DataTree constructor #9444

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

Merged
merged 6 commits into from
Sep 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
29 changes: 14 additions & 15 deletions xarray/core/datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,13 @@ def _collect_data_and_coord_variables(
return data_variables, coord_variables


def _coerce_to_dataset(data: Dataset | DataArray | None) -> Dataset:
if isinstance(data, DataArray):
ds = data.to_dataset()
elif isinstance(data, Dataset):
def _to_new_dataset(data: Dataset | None) -> Dataset:
if isinstance(data, Dataset):
ds = data.copy(deep=False)
elif data is None:
ds = Dataset()
else:
raise TypeError(
f"data object is not an xarray Dataset, DataArray, or None, it is of type {type(data)}"
)
raise TypeError(f"data object is not an xarray.Dataset, dict, or None: {data}")
return ds


Expand Down Expand Up @@ -422,7 +418,7 @@ class DataTree(

def __init__(
self,
data: Dataset | DataArray | None = None,
data: Dataset | None = None,
parent: DataTree | None = None,
children: Mapping[str, DataTree] | None = None,
name: str | None = None,
Expand All @@ -436,9 +432,8 @@ def __init__(

Parameters
----------
data : Dataset, DataArray, or None, optional
Data to store under the .ds attribute of this node. DataArrays will
be promoted to Datasets. Default is None.
data : Dataset, optional
Data to store under the .ds attribute of this node.
parent : DataTree, optional
Parent node to this node. Default is None.
children : Mapping[str, DataTree], optional
Expand All @@ -458,7 +453,7 @@ def __init__(
children = {}

super().__init__(name=name)
self._set_node_data(_coerce_to_dataset(data))
self._set_node_data(_to_new_dataset(data))
self.parent = parent
self.children = children

Expand Down Expand Up @@ -553,8 +548,8 @@ def ds(self) -> DatasetView:
return self._to_dataset_view(rebuild_dims=True)

@ds.setter
def ds(self, data: Dataset | DataArray | None = None) -> None:
ds = _coerce_to_dataset(data)
def ds(self, data: Dataset | None = None) -> None:
ds = _to_new_dataset(data)
self._replace_node(ds)

def to_dataset(self, inherited: bool = True) -> Dataset:
Expand Down Expand Up @@ -1096,8 +1091,12 @@ def from_dict(
if isinstance(root_data, DataTree):
obj = root_data.copy()
obj.orphan()
else:
elif root_data is None or isinstance(root_data, Dataset):
obj = cls(name=name, data=root_data, parent=None, children=None)
else:
raise TypeError(
f'root node data (at "/") must be a Dataset or DataTree, got {type(root_data)}'
)

def depth(item) -> int:
pathstr, _ = item
Expand Down
13 changes: 13 additions & 0 deletions xarray/tests/test_datatree.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ def test_bad_names(self):
with pytest.raises(ValueError):
DataTree(name="folder/data")

def test_data_arg(self):
ds = xr.Dataset({"foo": 42})
tree: DataTree = DataTree(data=ds)
assert_identical(tree.to_dataset(), ds)

with pytest.raises(TypeError):
DataTree(data=xr.DataArray(42, name="foo")) # type: ignore


class TestFamilyTree:
def test_setparent_unnamed_child_node_fails(self):
Expand Down Expand Up @@ -586,6 +594,11 @@ def test_insertion_order(self):
# despite 'Bart' coming before 'Lisa' when sorted alphabetically
assert list(reversed["Homer"].children.keys()) == ["Lisa", "Bart"]

def test_array_values(self):
data = {"foo": xr.DataArray(1, name="bar")}
with pytest.raises(TypeError):
DataTree.from_dict(data) # type: ignore


class TestDatasetView:
def test_view_contents(self):
Expand Down