fix: ED-2306 Make dynamic attributes unaware of spaces

encord/objects/ontology_labels_impl.py

@@ -2201,12 +2201,13 @@ def _to_object_actions(self) -> Dict[str, ObjectAction]:
             for obj in space._objects_map.values():
                 # Currently, dynamic attributes only available for VideoSpace
                 if isinstance(space, VideoSpace):
-                    all_static_answers = space._dynamic_answers_to_encord_dict(obj)
+                    all_static_answers = self._dynamic_answers_to_encord_dict(obj)
                     if len(all_static_answers) == 0:
                         continue
 
                     if obj.object_hash in ret:
-                        ret[obj.object_hash]["actions"].extend(list(all_static_answers))
+                        # The same object might still exist across object hashes
+                        continue
                     else:
                         ret[obj.object_hash] = {
                             "actions": list(all_static_answers),
@@ -3056,19 +3057,17 @@ def _add_action_answers(self, label_row_dict: dict):
         for answer in label_row_dict["object_actions"].values():
             object_hash = answer["objectHash"]
             object_instance = self._objects_map.get(object_hash)
-
+            answer_list = answer["actions"]
             if object_instance is not None:
-                answer_list = answer["actions"]
                 object_instance.set_answer_from_list(answer_list)
             else:
+                # Not great that we're looping through spaces, but usually not that many spaces on a label row
                 answer_list = answer["actions"]
-                for answer_dict in answer_list:
-                    space_id = answer_dict.get("spaceId")
-                    if space_id is None:
-                        raise LabelRowError("Object action does not contain spaceId")
-
-                    space = self.get_space(id=space_id, type_="video")
-                    space._set_answer_from_list(object_hash, answers_list=[answer_dict])
+                for space in self._space_map.values():
+                    object_on_space = space._objects_map.get(object_hash)
+                    if object_on_space is not None:
+                        object_on_space.set_answer_from_list(answers_list=answer_list)
+                        break
 
     def _create_new_object_instance(self, frame_object_label: FrameObject, frame: int) -> ObjectInstance:
         ontology = self._ontology.structure

encord/objects/ontology_object_instance.py

@@ -242,9 +242,6 @@ def get_answer(
             )
 
         if attribute.dynamic:
-            self._operation_not_allowed_for_objects_on_space(
-                extended_message="For getting dynamic attributes for objects on a space, use VideoSpace.get_answer_on_frames."
-            )
             return self._dynamic_answer_manager.get_answer(attribute, filter_answer, filter_frame)
 
         static_answer = self._static_answer_map[attribute.feature_node_hash]
@@ -298,10 +295,6 @@ def set_answer(
             )
         elif frames is not None and attribute.dynamic is False:
             raise LabelRowError("Setting frames is only possible for dynamic attributes.")
-        elif attribute.dynamic:
-            self._operation_not_allowed_for_objects_on_space(
-                extended_message="For setting dynamic attributes for objects on a space, use VideoSpace.set_answer_on_frames."
-            )
 
         if attribute.dynamic:
             self._dynamic_answer_manager.set_answer(answer, attribute, frames)

encord/objects/spaces/multiframe_space/multiframe_space.py

@@ -342,101 +342,13 @@ def _remove_all_answers_from_frames(self, object_instance: ObjectInstance, frame
             object_instance: The object instance to remove answers from.
             frames: List of frame numbers to remove answers from.
         """
-        dynamic_answer_manager = self._object_hash_to_dynamic_answer_manager.get(object_instance.object_hash)
-        if dynamic_answer_manager is None:
-            # No dynamic answers to remove
-            return
 
         # Get all dynamic attributes for this object
         dynamic_attributes = [attr for attr in object_instance._ontology_object.attributes if attr.dynamic]
 
         # Remove answers for each dynamic attribute on the specified frames
         for attribute in dynamic_attributes:
-            dynamic_answer_manager.delete_answer(attribute, frames=frames)
-
-    # This implementation is copied mostly from ObjectInstance.set_answer_from_list
-    def _set_answer_from_list(self, object_hash: str, answers_list: List[Dict[str, Any]]):
-        grouped_answers = defaultdict(list)
-        object_instance = self._objects_map[object_hash]
-        dynamic_answer_manager = self._object_hash_to_dynamic_answer_manager[object_instance.object_hash]
-
-        for answer_dict in answers_list:
-            attribute = _get_attribute_by_hash(answer_dict["featureHash"], object_instance._ontology_object.attributes)
-            if attribute is None:
-                raise LabelRowError(
-                    "One of the attributes does not exist in the ontology. Cannot create a valid LabelRow."
-                )
-            if not object_instance._is_attribute_valid_child_of_object_instance(attribute):
-                raise LabelRowError(
-                    "One of the attributes set for a classification is not a valid child of the classification. "
-                    "Cannot create a valid LabelRow."
-                )
-
-            grouped_answers[attribute.feature_node_hash].append(answer_dict)
-
-        for feature_hash, answers_list in grouped_answers.items():
-            attribute = _get_attribute_by_hash(feature_hash, object_instance._ontology_object.attributes)
-            assert attribute  # we already checked that attribute is not null above. So just silencing this for now
-            self._set_answer_from_grouped_list(dynamic_answer_manager, attribute, answers_list)
-
-    def _set_answer_from_grouped_list(
-        self, dynamic_answer_manager: DynamicAnswerManager, attribute: Attribute, answers_list: List[Dict[str, Any]]
-    ) -> None:
-        if isinstance(attribute, ChecklistAttribute):
-            if not attribute.dynamic:
-                raise LabelRowError("This method should not be called for non-dynamic attributes.")
-            else:
-                all_feature_hashes: Set[str] = set()
-                ranges = []
-                for answer_dict in answers_list:
-                    feature_hashes: Set[str] = {answer["featureHash"] for answer in answer_dict["answers"]}
-                    all_feature_hashes.update(feature_hashes)
-                    for frame_range in ranges_list_to_ranges(answer_dict["range"]):
-                        ranges.append((frame_range, feature_hashes))
-
-                options_cache = {
-                    feature_hash: attribute.get_child_by_hash(feature_hash, type_=Option)
-                    for feature_hash in all_feature_hashes
-                }
-
-                for frame_range, feature_hashes in ObjectInstance._merge_answers_to_non_overlapping_ranges(ranges):
-                    options = [options_cache[feature_hash] for feature_hash in feature_hashes]
-                    dynamic_answer_manager.set_answer(options, attribute, [frame_range])
-        else:
-            for answer in answers_list:
-                self._set_answer_from_dict(dynamic_answer_manager, answer, attribute)
-
-    def _set_answer_from_dict(
-        self, dynamic_answer_manager: DynamicAnswerManager, answer_dict: Dict[str, Any], attribute: Attribute
-    ) -> None:
-        if not attribute.dynamic:
-            raise LabelRowError("This method should not be called for non-dynamic attributes.")
-
-        ranges = ranges_list_to_ranges(answer_dict["range"])
-
-        if isinstance(attribute, TextAttribute):
-            dynamic_answer_manager.set_answer(answer_dict["answers"], attribute, ranges)
-        elif isinstance(attribute, RadioAttribute):
-            if len(answer_dict["answers"]) == 1:
-                feature_hash = answer_dict["answers"][0]["featureHash"]
-                option = attribute.get_child_by_hash(feature_hash, type_=Option)
-                dynamic_answer_manager.set_answer(option, attribute, ranges)
-        elif isinstance(attribute, ChecklistAttribute):
-            options = []
-            for answer in answer_dict["answers"]:
-                feature_hash = answer["featureHash"]
-                option = attribute.get_child_by_hash(feature_hash, type_=Option)
-                options.append(option)
-            dynamic_answer_manager.set_answer(options, attribute, ranges)
-        elif isinstance(attribute, NumericAttribute):
-            value: float = answer_dict["answers"]
-
-            if not isinstance(value, float) and not isinstance(value, int):
-                raise LabelRowError(f"The answer for a numeric attribute must be a float or an int. Found {value}.")
-
-            dynamic_answer_manager.set_answer(value, attribute, ranges)
-        else:
-            raise NotImplementedError(f"The attribute type {type(attribute)} is not supported.")
+            object_instance._dynamic_answer_manager.delete_answer(attribute, frames=frames)
 
     def set_dynamic_answer(
         self,
@@ -464,21 +376,21 @@ def set_dynamic_answer(
                 or if the object doesn't exist on the space yet.
         """
         self._label_row._check_labelling_is_initalised()
-        if attribute is None:
-            attribute = _infer_attribute_from_answer(object_instance._ontology_object.attributes, answer)
-        if not object_instance._is_attribute_valid_child_of_object_instance(attribute):
-            raise LabelRowError("The attribute is not a valid child of the object.")
-        elif not attribute.dynamic and not object_instance._is_selectable_child_attribute(attribute):
-            raise LabelRowError(
-                "Setting a nested attribute is only possible if all parent attributes have been selected."
-            )
-        elif attribute.dynamic is False:
+
+        if object_instance.object_hash not in self._objects_map:
             raise LabelRowError(
-                "This method should only be used for dynamic attributes. For static attributes, use `ObjectInstance.set_answer`."
+                "Object does not yet exist on this space. Place the object on this space with `Space.place_object`."
             )
 
+        if attribute is None:
+            attribute = _infer_attribute_from_answer(object_instance._ontology_object.attributes, answer)
+
+        if not attribute.dynamic:
+            raise LabelRowError("This method should not be called for non-dynamic attributes.")
+
         frames_list = frames_class_to_frames_list(frames)
 
+        # Check that frames do exist on this object on this space
         valid_frames = []
         for frame in frames_list:
             annotation_data = self._get_frame_object_annotation_data(
@@ -487,13 +399,7 @@ def set_dynamic_answer(
             if annotation_data is not None:
                 valid_frames.append(frame)
 
-        dynamic_answer_manager = self._object_hash_to_dynamic_answer_manager.get(object_instance.object_hash)
-        if dynamic_answer_manager is None:
-            raise LabelRowError(
-                "Object does not yet exist on this space. Place the object on this space with `Space.place_object`."
-            )
-
-        dynamic_answer_manager.set_answer(answer, attribute, frames=valid_frames)
+        object_instance.set_answer(answer, attribute, frames=valid_frames)
 
     def remove_dynamic_answer(
         self,
@@ -518,13 +424,7 @@ def remove_dynamic_answer(
         if not attribute.dynamic:
             raise LabelRowError("This method should not be called for non-dynamic attributes.")
 
-        dynamic_answer_manager = self._object_hash_to_dynamic_answer_manager.get(object_instance.object_hash)
-        if dynamic_answer_manager is None:
-            raise LabelRowError(
-                "Object does not yet exist on this space. Place the object on this space with `Space.place_object`."
-            )
-
-        dynamic_answer_manager.delete_answer(attribute, frames=frame, filter_answer=filter_answer)
+        object_instance._dynamic_answer_manager.delete_answer(attribute, frames=frame, filter_answer=filter_answer)
 
     def get_dynamic_answer(
         self,
@@ -557,7 +457,7 @@ def get_dynamic_answer(
         if not attribute.dynamic:
             raise LabelRowError("This method should only be used for dynamic attributes.")
 
-        return dynamic_answer_manager.get_answer(attribute, filter_answer, filter_frames=frames)
+        return object_instance._dynamic_answer_manager.get_answer(attribute, filter_answer, filter_frames=frames)
 
     def put_classification_instance(
         self,

tests/objects/data/data_group/two_videos.py

@@ -25,7 +25,7 @@
 }
 
 
-DATA_GROUP_METADATA = LabelRowMetadata(
+DATA_GROUP_WITH_TWO_VIDEOS_METADATA = LabelRowMetadata(
     label_hash="",
     branch_name="main",
     created_at=datetime.datetime.now(),
@@ -121,6 +121,12 @@
         "classification1": {
             "classificationHash": "classification1",
             "featureHash": "jPOcEsbw",
+            "spaces": {
+                "video-1-uuid": {
+                    "range": [[0, 0]],
+                    "type": "frame",
+                },
+            },
             "classifications": [
                 {
                     "name": "Text classification",
@@ -145,7 +151,7 @@
                     "featureHash": "OTkxMjU1",
                     "shouldPropagate": False,
                     "manualAnnotation": True,
-                    "spaceId": "video-1-uuid",
+                    "trackHash": "fbb97dda-1e66-48f9-b749-af2f83dab9fc",
                 },
                 {
                     "name": "First name",
@@ -156,7 +162,7 @@
                     "featureHash": "OTkxMjU1",
                     "shouldPropagate": False,
                     "manualAnnotation": True,
-                    "spaceId": "video-2-uuid",
+                    "trackHash": "fbb97dda-1e66-48f9-b749-af2f83dab9fc",
                 },
             ],
         },

tests/objects/spaces/test_multi_frame_space/test_dynamic_attributes.py

@@ -9,8 +9,8 @@
 from encord.objects.frames import Range
 from tests.objects.data.all_types_ontology_structure import all_types_structure
 from tests.objects.data.data_group.two_videos import (
-    DATA_GROUP_METADATA,
     DATA_GROUP_TWO_VIDEOS_NO_LABELS,
+    DATA_GROUP_WITH_TWO_VIDEOS_METADATA,
 )
 
 keypoint_with_dynamic_attributes_ontology_item = all_types_structure.get_child_by_hash("MTY2MTQx", Object)
@@ -21,7 +21,7 @@
 
 def test_add_dynamic_attributes_to_frames_on_object_on_video_space(ontology):
     # Arrange
-    label_row = LabelRowV2(DATA_GROUP_METADATA, Mock(), ontology)
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
     label_row.from_labels_dict(DATA_GROUP_TWO_VIDEOS_NO_LABELS)
     video_space_1 = label_row.get_space(id="video-1-uuid", type_="video")
 
@@ -36,7 +36,9 @@ def test_add_dynamic_attributes_to_frames_on_object_on_video_space(ontology):
     answer_on_frame_0 = "Frame 0"
     answer_on_frame_1_and_2 = "Frame 1 and 2"
 
-    # Act
+    new_object_instance.set_answer(frames=[0], attribute=key_point_dynamic_text_attribute, answer=answer_on_frame_0)
+
+    # # Act
     video_space_1.set_dynamic_answer(
         object_instance=new_object_instance,
         frames=[0],
@@ -49,8 +51,8 @@ def test_add_dynamic_attributes_to_frames_on_object_on_video_space(ontology):
         attribute=key_point_dynamic_text_attribute,
         answer=answer_on_frame_1_and_2,
     )
-
-    # Assert
+    #
+    # # Assert
     actual_answers = video_space_1.get_dynamic_answer(
         object_instance=new_object_instance,
         frames=[0, 1, 2],
@@ -70,7 +72,7 @@ def test_add_dynamic_attributes_to_frames_on_object_on_video_space(ontology):
 
 def test_remove_dynamic_attributes_from_frame_on_video_space(ontology):
     # Arrange
-    label_row = LabelRowV2(DATA_GROUP_METADATA, Mock(), ontology)
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
     label_row.from_labels_dict(DATA_GROUP_TWO_VIDEOS_NO_LABELS)
     video_space_1 = label_row.get_space(id="video-1-uuid", type_="video")
 
@@ -111,7 +113,7 @@ def test_remove_dynamic_attributes_from_frame_on_video_space(ontology):
 
 def test_remove_object_from_frame_removes_dynamic_attributes_from_those_frames(ontology):
     # Arrange
-    label_row = LabelRowV2(DATA_GROUP_METADATA, Mock(), ontology)
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
     label_row.from_labels_dict(DATA_GROUP_TWO_VIDEOS_NO_LABELS)
     video_space_1 = label_row.get_space(id="video-1-uuid", type_="video")
 
@@ -146,7 +148,7 @@ def test_remove_object_from_frame_removes_dynamic_attributes_from_those_frames(o
 
 def test_remove_object_removes_dynamic_attributes_for_that_object(ontology):
     # Arrange
-    label_row = LabelRowV2(DATA_GROUP_METADATA, Mock(), ontology)
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
     label_row.from_labels_dict(DATA_GROUP_TWO_VIDEOS_NO_LABELS)
     video_space_1 = label_row.get_space(id="video-1-uuid", type_="video")
 
@@ -177,7 +179,7 @@ def test_remove_object_removes_dynamic_attributes_for_that_object(ontology):
 
 def test_add_dynamic_attributes_to_frames_where_object_does_not_exist_on_video_space(ontology):
     # Arrange
-    label_row = LabelRowV2(DATA_GROUP_METADATA, Mock(), ontology)
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
     label_row.from_labels_dict(DATA_GROUP_TWO_VIDEOS_NO_LABELS)
     video_space_1 = label_row.get_space(id="video-1-uuid", type_="video")
 
@@ -212,7 +214,7 @@ def test_add_dynamic_attributes_to_frames_where_object_does_not_exist_on_video_s
 
 def test_add_dynamic_attributes_object_which_does_not_exist_on_video_space(ontology):
     # Arrange
-    label_row = LabelRowV2(DATA_GROUP_METADATA, Mock(), ontology)
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
     label_row.from_labels_dict(DATA_GROUP_TWO_VIDEOS_NO_LABELS)
     video_space_1 = label_row.get_space(id="video-1-uuid", type_="video")
 

tests/objects/spaces/test_serde.py

@@ -8,6 +8,10 @@
     DATA_GROUP_MULTILAYER_IMAGE_LABELS,
     DATA_GROUP_MULTILAYER_IMAGE_METADATA,
 )
+from tests.objects.data.data_group.two_videos import (
+    DATA_GROUP_WITH_TWO_VIDEOS_LABELS,
+    DATA_GROUP_WITH_TWO_VIDEOS_METADATA,
+)
 
 
 def test_read_and_export_all_space_labels(ontology):
@@ -153,3 +157,20 @@ def test_read_and_export_multilayer_image_labels(ontology):
         ],
         ignore_order_func=lambda x: x.path().endswith("['objects']"),
     )
+
+
+def test_read_and_export_video_group_with_dynamic_attributes(ontology):
+    label_row = LabelRowV2(DATA_GROUP_WITH_TWO_VIDEOS_METADATA, Mock(), ontology)
+    label_row.from_labels_dict(DATA_GROUP_WITH_TWO_VIDEOS_LABELS)
+
+    output_dict = label_row.to_encord_dict()
+
+    assert not DeepDiff(
+        DATA_GROUP_WITH_TWO_VIDEOS_LABELS,
+        output_dict,
+        exclude_regex_paths=[
+            r".*\['trackHash'\]",
+            r".*\['child_info'\]",  # We don't read this info in the BE
+        ],
+        ignore_order_func=lambda x: x.path().endswith("['objects']"),
+    )