[Possible bug] ContainsGroupError when trying to overwrite SpatialData object (overwriting a backed sdata/element) #137

Activity

1. melonora

   changed the title [-][Possible bug][/-] [+][Possible bug] ContainsGroupError when trying to overwrite SpatialData object[/+] on Feb 14, 2023
2. LucaMarconato

   added

   bug 🚨Something isn't workingSomething isn't working

   I/O 💿

   on Feb 14, 2023
3. LucaMarconato commented on Feb 14, 2023

   

   LucaMarconato

   on Feb 14, 2023

   Member

   More actions

   Hi, thanks for reporting. The errors is due to the fact that when you save a SpatialData object to disk, and when it contains backed elements (Images, Labels, Points), it doesn't load the data in-memory by default but it reads it lazily from disk.

   More generally, for the same reason, this happens even when you want to add a single element to an existing SpatialData object with add_image(), add_labels() and add_points().

   This is shown in the following example. All works with Shapes (Polygons and Tables), but the equivalent code breaks with Points.

   from spatialdata import SpatialData
   from tests.conftest import _get_shapes, _get_points
   import tempfile
   import os
   
   with tempfile.TemporaryDirectory() as tmpdir:
       f = os.path.join(tmpdir, "data.zarr")
       sdata = SpatialData(shapes=_get_shapes())
       print(sdata)
       sdata.write(f)
       e = sdata.shapes['shapes_0']
       sdata.add_shapes('new_shapes', e, overwrite=True)
   
       # ok
       sdata.add_shapes('new_shapes', e, overwrite=True)
       # ok, beacause shapes are not backed
       sdata.add_shapes('new_shapes', sdata.shapes['new_shapes'], overwrite=True)
   
   with tempfile.TemporaryDirectory() as tmpdir:
       f = os.path.join(tmpdir, "data.zarr")
       sdata = SpatialData(points=_get_points())
       print(sdata)
       sdata.write(f)
       e = sdata.points['points_0']
       sdata.add_points('new_points', e, overwrite=True)
   
       # ok
       sdata.add_points('new_points', e, overwrite=True)
       # exception (data is deleted, we should raise an error before this happens)
       sdata.add_points('new_points', sdata.points['new_points'], overwrite=True)
   

   In the latest version from main (I suggest doing a develop install of the repo so that you can install the latest version by just pulling the repo (or changing branch)), the error that you got should not happen: you should get an exception before it happens.

   Traceback (most recent call last):
     File "/Users/macbook/Library/Application Support/JetBrains/PyCharm2022.3/scratches/scratch_13.py", line 22, in <module>
       sdata.write(f, overwrite=True)
     File "/Users/macbook/embl/projects/basel/spatialdata/spatialdata/_core/_spatialdata.py", line 663, in write
       raise ValueError("Can't overwrite the original file")
   ValueError: Can't overwrite the original file
   

   But for add_image(), add_labels() and add_points() no exception is raised and this leads to data loss. This is a critical bug and I'll address it soon.
4. LucaMarconato commented on Feb 14, 2023

   

   LucaMarconato

   on Feb 14, 2023

   Member

   More actions

   In the near future we will allow the user to rewrite a backed object in-place. An implementation could be to write the new zarr group in a temporary directory and then do an atomic move of the new data to the original one. In this way we also prevent data loss if something goes wrong in between.

   For the moment, a quick and dirty workaround is to save the SpatialData object in a new location, and then the .zarr to the previous location, basically a manual version of the above (or feel free to make a pr).
5. LucaMarconato

   changed the title [-][Possible bug] ContainsGroupError when trying to overwrite SpatialData object[/-] [+][Possible bug] ContainsGroupError when trying to overwrite SpatialData object (overwriting a backed sdata/element)[/+] on Feb 14, 2023
6. LucaMarconato commented on Feb 14, 2023

   

   LucaMarconato

   on Feb 14, 2023

   Last edited by LucaMarconato

   Member

   More actions

   Actually I have fixed the bug your encountered and the one that I described, in this PR #138. I used the approach discussed above of writing to another location and then automatically moving the written data. All the test passes, please also check it if works.

   Thanks again for reporting this critical bug.
7. melonora commented on Feb 14, 2023

   

   melonora

   on Feb 14, 2023

   CollaboratorAuthor

   More actions

   Thanks for looking into it. I will check!
8. melonora commented on Feb 17, 2023

   

   melonora

   on Feb 17, 2023

   CollaboratorAuthor

   More actions

   I can confirm that on main no group error is given anymore and properly overwrites. I checked this by also given a different number of images when overwriting. If you are ok with it, this issue can be closed.
9. LucaMarconato commented on Feb 27, 2023

   

   LucaMarconato

   on Feb 27, 2023

   Member

   More actions

   As discussed in this PR: #138 we decided not to add the support for overwriting the data into the path that is used for backing. This holds true both for SpatialData objects and backed elements (images, labels and points). I kindly invite you and any other users reading this to provide feedback regarding this behavior, since we want to get back to it in the future. Closing for now.
10. LucaMarconato

    closed this as completedon Feb 27, 2023