Exceptions when adding labels to backed SpatialData

aeisenbarth commented 1 year ago

When saving labels to a SpatialData that has already been written to disk (with or without overwrite=True), there are multiple exceptions.

Example code

import numpy as np
import spatialdata

sdata = spatialdata.SpatialData()
sdata.write("/tmp/spatialdata.zarr")
labels = spatialdata.models.Labels2DModel.parse(np.ones((2, 2)))
sdata.add_labels(name="my_label", labels=labels, overwrite=True)

1. UnboundLocalError: local variable 'elem_group' referenced before assignment

A variable is accessed (L415) which is only conditionally declared. It would be safer either to assign a default value, or to access it only in the same conditional branch.

I don't understand the reasoning of the comment, but to me it seems a fix would be to remove the condition above L411.

https://github.com/scverse/spatialdata/blob/4dd525002f12381858251df9cd304a7108a21886/src/spatialdata/_core/spatialdata.py#L409-L416

Traceback (most recent call last):
  File "<ipython-input-6-6d9e217edb59>", line 1, in <module>
    sdata.add_labels(name="my_label", labels=labels, overwrite=True)
  File "…/spatialdata/_core/spatialdata.py", line 779, in add_labels
    elem_group = self._init_add_element(name=name, element_type="labels", overwrite=overwrite)
  File "…/spatialdata/_core/spatialdata.py", line 415, in _init_add_element
    if name in elem_group:
UnboundLocalError: local variable 'elem_group' referenced before assignment

2. AssertionError on type of dims

For backed labels, xdata is not a DataArray (where dims is ("y", "x")) but a DataTree where dims is Frozen({"y": 1633, "x": 1290}).

https://github.com/scverse/spatialdata/blob/4dd525002f12381858251df9cd304a7108a21886/src/spatialdata/models/_utils.py#L151-L153

This works with:

xdata = next(iter(e["scale0"].ds.values()))

or converting the dictionary to tuple:

        if isinstance(dims_data, Mapping):
            dims_data = tuple(dims_data.keys())

3. DataTree has no attribute `shape`

https://github.com/scverse/spatialdata/blob/4dd525002f12381858251df9cd304a7108a21886/src/spatialdata/transformations/_utils.py#L118-L119

Also here:

        xdata = next(iter(e["scale0"].ds.values()))

LucaMarconato commented 1 year ago

Thanks for reporting, I will look into this and continue from your PR.

LucaMarconato commented 9 months ago

Closing because the IO apis changed in main with https://github.com/scverse/spatialdata/pull/329 (please note that the PR is not in the current version v0.0.15).

I have opened a separate issue to track the other points https://github.com/scverse/spatialdata/issues/446.

scverse / spatialdata