[SYNPY-1417] Updates for the annotation model (Sage-Bionetworks#1081)

Finishing touches for the annotation model. Defaulting to empty dict.

Author: BryanFauble

synapseclient/api/annotations.py

@@ -23,14 +23,18 @@ def set_annotations(
     """Call to synapse and set the annotations for the given input.
 
     Arguments:
-        annotations: The annotations to set. This is expected to have the id, etag, and annotations filled in.
-        synapse_client: If not passed in or None this will use the last client from the `.login()` method.
+        annotations: The annotations to set. This is expected to have the id, etag,
+            and annotations filled in.
+        synapse_client: If not passed in or None this will use the last client from
+            the `.login()` method.
 
     Returns: The annotations set in Synapse.
     """
     annotations_dict = asdict(annotations)
 
-    synapse_annotations = _convert_to_annotations_list(annotations_dict["annotations"])
+    synapse_annotations = _convert_to_annotations_list(
+        annotations_dict["annotations"] or {}
+    )
     from synapseclient import Synapse
 
     return Synapse.get_client(synapse_client=synapse_client).restPUT(

synapseclient/models/annotations.py

@@ -1,5 +1,5 @@
 import asyncio
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from datetime import date, datetime
 from typing import Dict, List, Optional, Union
 
@@ -48,7 +48,7 @@ class Annotations(AnnotationsSynchronousProtocol):
             ],
         ],
         None,
-    ]
+    ] = field(default_factory=dict)
     """Additional metadata associated with the object. The key is the name of your
     desired annotations. The value is an object containing a list of string values
     (use empty list to represent no values for key) and the value type associated with
@@ -145,4 +145,4 @@ def from_dict(
             else:
                 annotations[key] = dict_to_convert[key]
 
-        return annotations if annotations else None
+        return annotations

synapseclient/models/file.py

@@ -207,7 +207,7 @@ class File(FileSynchronousProtocol, AccessControllable):
             of your desired annotations. The value is an object containing a list of
             values (use empty list to represent no values for key) and the value type
             associated with all values in the list. To remove all annotations set this
-            to an empty dict `{}`.
+            to an empty dict `{}` or None and store the entity.
 
     Attributes:
         content_type: (New Upload Only)
@@ -349,7 +349,7 @@ class File(FileSynchronousProtocol, AccessControllable):
                 List[datetime],
             ],
         ]
-    ] = field(default=None, compare=False)
+    ] = field(default_factory=dict, compare=False)
     """Additional metadata associated with the folder. The key is the name of your
     desired annotations. The value is an object containing a list of values
     (use empty list to represent no values for key) and the value type associated with
@@ -512,7 +512,7 @@ def _set_last_persistent_instance(self) -> None:
             dataclasses.replace(self.activity) if self.activity else None
         )
         self._last_persistent_instance.annotations = (
-            deepcopy(self.annotations) if self.annotations else None
+            deepcopy(self.annotations) if self.annotations else {}
         )
 
     def _fill_from_file_handle(self) -> None:
@@ -562,7 +562,7 @@ def fill_from_dict(
 
         if set_annotations:
             self.annotations = Annotations.from_dict(
-                synapse_file.get("annotations", None)
+                synapse_file.get("annotations", {})
             )
         return self
 

synapseclient/models/folder.py

@@ -58,8 +58,8 @@ class Folder(FolderSynchronousProtocol, AccessControllable, StorableContainer):
         annotations: Additional metadata associated with the folder. The key is the name
             of your desired annotations. The value is an object containing a list of
             values (use empty list to represent no values for key) and the value type
-            associated with all values in the list.  To remove all annotations set this
-            to an empty dict `{}`.
+            associated with all values in the list. To remove all annotations set this
+            to an empty dict `{}` or None and store the entity.
         create_or_update: (Store only) Indicates whether the method should
             automatically perform an update if the resource conflicts with an existing
             Synapse object. When True this means that any changes to the resource will
@@ -124,11 +124,12 @@ class Folder(FolderSynchronousProtocol, AccessControllable, StorableContainer):
                 List[datetime],
             ],
         ]
-    ] = field(default=None, compare=False)
+    ] = field(default_factory=dict, compare=False)
     """Additional metadata associated with the folder. The key is the name of your
     desired annotations. The value is an object containing a list of values
     (use empty list to represent no values for key) and the value type associated with
-    all values in the list. To remove all annotations set this to an empty dict `{}`."""
+    all values in the list. To remove all annotations set this to an empty dict `{}`
+    or None and store the entity."""
 
     is_restricted: bool = field(default=False, repr=False)
     """
@@ -173,7 +174,7 @@ def _set_last_persistent_instance(self) -> None:
         del self._last_persistent_instance
         self._last_persistent_instance = replace(self)
         self._last_persistent_instance.annotations = (
-            deepcopy(self.annotations) if self.annotations else None
+            deepcopy(self.annotations) if self.annotations else {}
         )
 
     def fill_from_dict(

synapseclient/models/project.py

@@ -54,8 +54,8 @@ class Project(ProjectSynchronousProtocol, AccessControllable, StorableContainer)
         annotations: Additional metadata associated with the folder. The key is the name
             of your desired annotations. The value is an object containing a list of
             values (use empty list to represent no values for key) and the value type
-            associated with all values in the list.  To remove all annotations set this
-            to an empty dict `{}`.
+            associated with all values in the list. To remove all annotations set this
+            to an empty dict `{}` or None and store the entity.
         create_or_update: (Store only) Indicates whether the method should
             automatically perform an update if the resource conflicts with an existing
             Synapse object. When True this means that any changes to the resource will
@@ -157,11 +157,12 @@ class Project(ProjectSynchronousProtocol, AccessControllable, StorableContainer)
                 List[datetime],
             ],
         ]
-    ] = field(default=None, compare=False)
+    ] = field(default_factory=dict, compare=False)
     """Additional metadata associated with the folder. The key is the name of your
     desired annotations. The value is an object containing a list of values
     (use empty list to represent no values for key) and the value type associated with
-    all values in the list. To remove all annotations set this to an empty dict `{}`."""
+    all values in the list. To remove all annotations set this to an empty dict `{}`
+    or None and store the entity."""
 
     create_or_update: bool = field(default=True, repr=False)
     """
@@ -200,7 +201,7 @@ def _set_last_persistent_instance(self) -> None:
         del self._last_persistent_instance
         self._last_persistent_instance = replace(self)
         self._last_persistent_instance.annotations = (
-            deepcopy(self.annotations) if self.annotations else None
+            deepcopy(self.annotations) if self.annotations else {}
         )
 
     def fill_from_dict(
@@ -229,7 +230,7 @@ def fill_from_dict(
         self.parent_id = synapse_project.get("parentId", None)
         if set_annotations:
             self.annotations = Annotations.from_dict(
-                synapse_project.get("annotations", None)
+                synapse_project.get("annotations", {})
             )
         return self
 

synapseclient/models/services/storable_entity_components.py

@@ -186,7 +186,7 @@ async def _store_activity_and_annotations(
     )
     if (
         hasattr(root_resource, "annotations")
-        and root_resource.annotations is not None
+        and (root_resource.annotations or last_persistent_instance)
         and (
             last_persistent_instance is None
             or last_persistent_instance.annotations != root_resource.annotations

synapseclient/models/table.py

@@ -1,6 +1,6 @@
 import asyncio
 import os
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from datetime import date, datetime
 from enum import Enum
 from typing import Any, Dict, List, Optional, Union
@@ -376,10 +376,11 @@ class Table(TableSynchronousProtocol, AccessControllable):
         is_search_enabled: When creating or updating a table or view specifies if full
             text search should be enabled. Note that enabling full text search might
             slow down the indexing of the table or view.
-        annotations: Additional metadata associated with the table. The key is the
-            name of your desired annotations. The value is an object containing a list
-            of values (use empty list to represent no values for key) and the value
-            type associated with all values in the list.
+        annotations: Additional metadata associated with the table. The key is the name
+            of your desired annotations. The value is an object containing a list of
+            values (use empty list to represent no values for key) and the value type
+            associated with all values in the list. To remove all annotations set this
+            to an empty dict `{}` or None and store the entity.
 
     """
 
@@ -454,11 +455,12 @@ class Table(TableSynchronousProtocol, AccessControllable):
                 List[datetime],
             ],
         ]
-    ] = None
+    ] = field(default_factory=dict)
     """Additional metadata associated with the table. The key is the name of your
     desired annotations. The value is an object containing a list of values
     (use empty list to represent no values for key) and the value type associated with
-    all values in the list. To remove all annotations set this to an empty dict `{}`."""
+    all values in the list. To remove all annotations set this to an empty dict `{}`
+    or None and store the entity."""
 
     def fill_from_dict(
         self, synapse_table: Synapse_Table, set_annotations: bool = True
@@ -488,7 +490,7 @@ def fill_from_dict(
         ]
         if set_annotations:
             self.annotations = Annotations.from_dict(
-                synapse_table.get("annotations", None)
+                synapse_table.get("annotations", {})
             )
         return self
 

tests/integration/synapseclient/models/async/test_file_async.py

@@ -413,6 +413,122 @@ async def test_store_annotations(self, project_model: Project, file: File) -> No
         assert file.annotations["my_key_double"] == [1.2, 3.4, 5.6]
         assert file.annotations["my_key_long"] == [1, 2, 3]
 
+    @pytest.mark.asyncio
+    async def test_setting_annotations_directly(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN a file with the annotation
+        file.name = str(uuid.uuid4())
+        file.annotations["my_single_key_string"] = "a"
+        file.annotations["my_key_string"] = ["b", "a", "c"]
+        file.annotations["my_key_bool"] = [False, False, False]
+        file.annotations["my_key_double"] = [1.2, 3.4, 5.6]
+        file.annotations["my_key_long"] = [1, 2, 3]
+
+        # WHEN I store the file
+        file = await file.store_async(parent=project_model)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file annotations to have been stored
+        assert len(file.annotations.keys()) == 5
+        assert file.annotations["my_single_key_string"] == ["a"]
+        assert file.annotations["my_key_string"] == ["b", "a", "c"]
+        assert file.annotations["my_key_bool"] == [False, False, False]
+        assert file.annotations["my_key_double"] == [1.2, 3.4, 5.6]
+        assert file.annotations["my_key_long"] == [1, 2, 3]
+
+        # WHEN I update the annotations and store the file again
+        file.annotations["my_key_string"] = ["new", "values", "here"]
+        await file.store_async()
+
+        # THEN I expect the file annotations to have been updated
+        assert len(file.annotations.keys()) == 5
+        assert file.annotations["my_single_key_string"] == ["a"]
+        assert file.annotations["my_key_string"] == ["new", "values", "here"]
+        assert file.annotations["my_key_bool"] == [False, False, False]
+        assert file.annotations["my_key_double"] == [1.2, 3.4, 5.6]
+        assert file.annotations["my_key_long"] == [1, 2, 3]
+
+    @pytest.mark.asyncio
+    async def test_removing_annotations_to_none(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN an annotation
+        annotations_for_my_file = {
+            "my_single_key_string": "a",
+            "my_key_string": ["b", "a", "c"],
+            "my_key_bool": [False, False, False],
+            "my_key_double": [1.2, 3.4, 5.6],
+            "my_key_long": [1, 2, 3],
+        }
+
+        # AND a file with the annotation
+        file.name = str(uuid.uuid4())
+        file.annotations = annotations_for_my_file
+
+        # WHEN I store the file
+        file = await file.store_async(parent=project_model)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file annotations to have been stored
+        assert file.annotations.keys() == annotations_for_my_file.keys()
+        assert file.annotations["my_single_key_string"] == ["a"]
+        assert file.annotations["my_key_string"] == ["b", "a", "c"]
+        assert file.annotations["my_key_bool"] == [False, False, False]
+        assert file.annotations["my_key_double"] == [1.2, 3.4, 5.6]
+        assert file.annotations["my_key_long"] == [1, 2, 3]
+
+        # WHEN I update the annotations to None
+        file.annotations = None
+        await file.store_async()
+
+        # THEN I expect the file annotations to have been removed
+        assert not file.annotations and isinstance(file.annotations, dict)
+
+        # AND retrieving the file gives an empty dict for the annoations
+        file_copy = await File(id=file.id, download_file=False).get_async()
+        assert not file_copy.annotations and isinstance(file_copy.annotations, dict)
+
+    @pytest.mark.asyncio
+    async def test_removing_annotations_to_empty_dict(
+        self, project_model: Project, file: File
+    ) -> None:
+        # GIVEN an annotation
+        annotations_for_my_file = {
+            "my_single_key_string": "a",
+            "my_key_string": ["b", "a", "c"],
+            "my_key_bool": [False, False, False],
+            "my_key_double": [1.2, 3.4, 5.6],
+            "my_key_long": [1, 2, 3],
+        }
+
+        # AND a file with the annotation
+        file.name = str(uuid.uuid4())
+        file.annotations = annotations_for_my_file
+
+        # WHEN I store the file
+        file = await file.store_async(parent=project_model)
+        self.schedule_for_cleanup(file.id)
+
+        # THEN I expect the file annotations to have been stored
+        assert file.annotations.keys() == annotations_for_my_file.keys()
+        assert file.annotations["my_single_key_string"] == ["a"]
+        assert file.annotations["my_key_string"] == ["b", "a", "c"]
+        assert file.annotations["my_key_bool"] == [False, False, False]
+        assert file.annotations["my_key_double"] == [1.2, 3.4, 5.6]
+        assert file.annotations["my_key_long"] == [1, 2, 3]
+
+        # WHEN I update the annotations to an empty dict
+        file.annotations = {}
+        await file.store_async()
+
+        # THEN I expect the file annotations to have been removed
+        assert not file.annotations and isinstance(file.annotations, dict)
+
+        # AND retrieving the file gives an empty dict for the annoations
+        file_copy = await File(id=file.id, download_file=False).get_async()
+        assert not file_copy.annotations and isinstance(file_copy.annotations, dict)
+
     @pytest.mark.asyncio
     async def test_store_without_upload(
         self, project_model: Project, file: File
@@ -1453,7 +1569,7 @@ async def test_copy_activity_only(self, file: File) -> None:
 
         # THEN I expect the activities to be the same and annotations on the second to be None
         assert file_1.annotations != file_2.annotations
-        assert file_2.annotations is None
+        assert not file_2.annotations and isinstance(file_2.annotations, dict)
         assert file_1.activity == file_2.activity
 
     @pytest.mark.asyncio
@@ -1505,7 +1621,7 @@ async def test_copy_with_no_activity_or_annotations(self, file: File) -> None:
         # THEN I expect the activities to be the same and annotations on the second to be None
         assert file_1.annotations != file_2.annotations
         assert file_1.activity != file_2.activity
-        assert file_2.annotations is None
+        assert not file_2.annotations and isinstance(file_2.annotations, dict)
         assert file_2.activity is None
 
     @pytest.mark.asyncio