`get_elements` fails on empty SpatialData

[aeisenbarth]

When SpatialData has no elements and I select no elements from it, I expected it to be a valid operation. However, an error is raised:

from spatialdata import SpatialData
import spatialdata_plot
SpatialData().pp.get_elements(elements=[])
# ValueError: SpatialData object must have at least one coordinate system to generate a mapping.

This is because the implementation tries to get coordinate system names, which are bound on the existence of elements.

While it is unlikely users will want this rare use case, when I test my own code systematically with a matrix of edge cases, this issue makes my tests fail and I have to add workarounds.

[aeisenbarth]

An alternative solution is to remove the exception in _get_coordinate_system_mapping. I didn't want to touch it first, but when I think about it, an empty mapping should be a valid return value.

[aeisenbarth]

I would have preferred to add the test case as an additional parametrization of test_can_subset_to_one_or_more_images (then we would also need to rename it to …zero_or_more…). Unfortunately, it cannot use the more flexible sdata fixture with param="empty", only get_sdata_with_multiple_images. It is also confusing that the fixture value (called "sdata") is a function that needs to be called to get "sdata".

It's an interesting workaround to the problem of selecting a fixture by name and parametrizing it, since the sdata fixture only takes a string as request param. My approach to multiple parameters in requests is to use a dictionary. That way we could rewrite the first test function to:

# conftest.py
@pytest.fixture
…
def sdata(request) -> SpatialData:
    request.param: dict
    name = request.param.pop("name") # TODO: here we would need to provide a default
    if name == "full":
        …
    else:
        s = request.getfixturevalue(name)
        if callable(s):
            kwargs = request.param
            s = s(**kwargs)
    # print(f"request.param = {request.param}")
    return s

# test_pp.py
@pytest.mark.parametrize(
    "sdata, keys",
    [
        (dict(name="empty"), []),
        (dict(name="get_sdata_with_multiple_images", share_coordinate_system="all"), "data1"),
        (dict(name="get_sdata_with_multiple_images", share_coordinate_system="all"), ["data1"]),
        (dict(name="get_sdata_with_multiple_images", share_coordinate_system="all"), ["data1", "data2"]),
    ],
    indirect=["sdata"]
)
def test_can_subset_to_zero_or_more_images(sdata, keys):
    """Tests whether a subset of images can be selected from the sdata object."""
    clipped_sdata = sdata.pp.get_elements(keys)

    assert list(clipped_sdata.images.keys()) == ([keys] if isinstance(keys, str) else keys)

Note that the "sdata" is now not the name for the get_sdata_with_multiple_images but it is the actual sdatafixture, which give us more flexibility choosing and parametrizing other fixtures. But maybe there is even another approach.

[LucaMarconato]

Hi @aeisenbarth, I leave the conftest.py question to the others and I comment the first two posts.

I have just added an API that is similar to get_elements() (ideally a refactored version of get_elements() would be upstream, see here https://github.com/scverse/spatialdata-plot/issues/138), please check it out, maybe it's what you need: https://github.com/scverse/spatialdata/pull/426.

[timtreis]

Agreeing with Luca here, moving get_elements() upstream (and with it the entire pp accessor) was on our roadmap for quite a bit now, just with low priority. I think I wouldn't really be worth fixing it here for now.

[LucaMarconato]

I believe now get_elements() is not used anymore, is this correct? In that case the bug reported maybe be gone.

[aeisenbarth]

Yes, for myself, this is obsolete now and can be closed.