diff --git a/darwin/future/data_objects/properties.py b/darwin/future/data_objects/properties.py index d9e56f390..ac9661bfd 100644 --- a/darwin/future/data_objects/properties.py +++ b/darwin/future/data_objects/properties.py @@ -93,10 +93,11 @@ def to_create_endpoint( "name": True, "type": True, "required": True, - "property_values": {"__all__": {"value", "color", "type"}}, "description": True, "granularity": True, } + if self.type != "text": + include_fields["property_values"] = {"__all__": {"value", "color", "type"}} if self.granularity != PropertyGranularity.item: if self.annotation_class_id is None: raise ValueError("annotation_class_id must be set") diff --git a/darwin/importer/importer.py b/darwin/importer/importer.py index 69b015e63..a3fca9eb5 100644 --- a/darwin/importer/importer.py +++ b/darwin/importer/importer.py @@ -385,14 +385,21 @@ def _serialize_item_level_properties( for item_property_value in item_property_values: item_property = team_item_properties_lookup[item_property_value["name"]] item_property_id = item_property.id - item_property_value_id = next( - ( - pv.id - for pv in item_property.property_values or [] - if pv.value == item_property_value["value"] - ), - None, - ) + if ( + item_property.type == "single_select" + or item_property.type == "multi_select" + ): + item_property_value_id = next( + ( + pv.id + for pv in item_property.property_values or [] + if pv.value == item_property_value["value"] + ), + None, + ) + value = {"id": item_property_value_id} + elif item_property.type == "text": + value = {"text": item_property_value["value"]} actors: List[dt.DictFreeForm] = [] actors.extend( _handle_annotators( @@ -406,7 +413,7 @@ def _serialize_item_level_properties( { "actors": actors, "property_id": item_property_id, - "value": {"id": item_property_value_id}, + "value": value, } ) @@ -798,39 +805,19 @@ def _import_properties( _get_team_properties_annotation_lookup(client, dataset.team) ) - # Create or update item-level properties from annotations - item_property_creations_from_metadata, item_property_updates_from_metadata = ( - _create_update_item_properties( - _normalize_item_properties(item_properties), - team_item_properties_lookup, - client, - ) + # Update item-level properties from annotations + _, item_properties_to_update_from_annotations = _create_update_item_properties( + _normalize_item_properties(item_properties), + team_item_properties_lookup, + client, ) - properties_to_create = item_property_creations_from_metadata - properties_to_update = item_property_updates_from_metadata - - if properties_to_create: - console.print(f"Creating {len(properties_to_create)} properties:", style="info") - for full_property in properties_to_create: - if full_property.granularity.value == "item": - console.print( - f"- Creating item-level property '{full_property.name}' of type: {full_property.type}" - ) - console.print( - f"- Creating property '{full_property.name}' of type {full_property.type}", - ) - prop = client.create_property( - team_slug=full_property.slug, params=full_property - ) - created_properties.append(prop) - - if item_property_updates_from_metadata: + if item_properties_to_update_from_annotations: console.print( - f"Performing {len(item_property_updates_from_metadata)} property update(s):", + f"Performing {len(item_properties_to_update_from_annotations)} property update(s):", style="info", ) - for full_property in item_property_updates_from_metadata: + for full_property in item_properties_to_update_from_annotations: if full_property.granularity.value == "item": console.print( f"- Updating item-level property '{full_property.name}' with new value: {full_property.property_values[0].value}" @@ -966,7 +953,8 @@ def _normalize_item_properties( for item_prop in item_properties: name = item_prop["name"] value = item_prop["value"] - normalized_properties[name]["property_values"].append({"value": value}) + if value: + normalized_properties[name]["property_values"].append({"value": value}) return normalized_properties @@ -996,7 +984,11 @@ def _create_update_item_properties( # If the property exists in the team, check that all values are present if item_prop_name in team_item_properties_lookup: + t_prop = team_item_properties_lookup[item_prop_name] + # If the property is a text property it won't have predefined values, so continue + if t_prop.type == "text": + continue t_prop_values = [ prop_val.value for prop_val in t_prop.property_values or [] ] diff --git a/darwin/path_utils.py b/darwin/path_utils.py index 2f573596f..ffb2b1d4d 100644 --- a/darwin/path_utils.py +++ b/darwin/path_utils.py @@ -122,7 +122,10 @@ def is_properties_enabled( if _cls.get("properties"): return metadata_path for _item_level_property in metadata_item_level_properties: - if _item_level_property.get("property_values"): + if ( + _item_level_property.get("property_values") + or _item_level_property["type"] == "text" + ): return metadata_path return False diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_1.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_1.json index 4e1b58903..65da1a963 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_1.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_1.json @@ -590,6 +590,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_2.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_2.json index ce84acc81..4feec191e 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_2.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_2.json @@ -482,6 +482,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_3.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_3.json index 74928f6a2..fe3e37855 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_3.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_3.json @@ -526,6 +526,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_4.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_4.json index 994839a51..13101ba83 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_4.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_4.json @@ -578,6 +578,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_5.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_5.json index 2195af073..81e606669 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_5.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_5.json @@ -450,6 +450,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_6.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_6.json index 446c5d062..8e924e382 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_6.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_6.json @@ -462,6 +462,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_7.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_7.json index 042a37133..a118205fc 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_7.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_7.json @@ -502,6 +502,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_8.json b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_8.json index f7fe05564..406706c7a 100644 --- a/e2e_tests/data/import/image_annotations_with_item_level_properties/image_8.json +++ b/e2e_tests/data/import/image_annotations_with_item_level_properties/image_8.json @@ -478,6 +478,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "test_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/.v7/metadata.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/.v7/metadata.json index bdbe11d27..f296760a5 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/.v7/metadata.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/.v7/metadata.json @@ -89,6 +89,14 @@ } ], "granularity": "item" + }, + { + "name": "new_item_level_property_text", + "type": "text", + "description": "", + "required": false, + "property_values": [], + "granularity": "item" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_1.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_1.json index 68977035f..81340c5da 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_1.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_1.json @@ -590,6 +590,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_2.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_2.json index 4c493508c..f11a3432a 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_2.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_2.json @@ -482,6 +482,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_3.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_3.json index 4a119ccda..9e4a699aa 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_3.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_3.json @@ -526,6 +526,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_4.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_4.json index 449692551..fd71b89a4 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_4.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_4.json @@ -578,6 +578,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_5.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_5.json index 2195af073..a9bcba7d8 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_5.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_5.json @@ -450,6 +450,10 @@ { "name": "test_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_6.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_6.json index 2b14415b8..afdce5011 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_6.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_6.json @@ -462,6 +462,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_7.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_7.json index 42b1d8a20..bcc8a3e44 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_7.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_7.json @@ -502,6 +502,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_8.json b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_8.json index edf2143dc..6fea144e9 100644 --- a/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_8.json +++ b/e2e_tests/data/import/image_new_annotations_with_item_level_properties/image_8.json @@ -478,6 +478,10 @@ { "name": "new_item_level_property_single_select", "value": "1" + }, + { + "name": "new_item_level_property_text", + "value": "sample text" } ] } \ No newline at end of file diff --git a/e2e_tests/setup_tests.py b/e2e_tests/setup_tests.py index e5cc543cb..7ad4daed7 100644 --- a/e2e_tests/setup_tests.py +++ b/e2e_tests/setup_tests.py @@ -284,11 +284,12 @@ def create_item_level_property( "name": name, "type": item_level_property_type, "granularity": "item", - "property_values": [ + } + if item_level_property_type in ["single_select", "multi_select"]: + payload["property_values"] = [ {"color": "rgba(255,92,0,1.0)", "value": "1"}, {"color": "rgba(255,92,0,1.0)", "value": "2"}, - ], - } + ] response = requests.post(url, json=payload, headers=headers) parsed_response = response.json() return E2EItemLevelProperty( @@ -593,7 +594,7 @@ def setup_item_level_properties(config: ConfigValues) -> List[E2EItemLevelProper item_level_properties: List[E2EItemLevelProperty] = [] print("Setting up item-level properties") - item_level_property_types = ["single_select", "multi_select"] + item_level_property_types = ["single_select", "multi_select", "text"] try: for item_level_property_type in item_level_property_types: try: diff --git a/tests/darwin/importer/importer_test.py b/tests/darwin/importer/importer_test.py index 8eb16cb67..74bf7e7ac 100644 --- a/tests/darwin/importer/importer_test.py +++ b/tests/darwin/importer/importer_test.py @@ -33,6 +33,7 @@ _verify_slot_annotation_alignment, _import_properties, _warn_for_annotations_with_multiple_instance_ids, + _serialize_item_level_properties, ) @@ -3278,3 +3279,197 @@ def test_warning_with_multiple_files_with_multi_instance_id_annotations(): assert ( console.print.call_count == 3 ) # One for the warning message, two for the file details + + +def test_serialize_item_level_properties_empty_input(): + """Test that empty input returns empty list""" + result = _serialize_item_level_properties([], Mock(), Mock(), False, False) + assert result == [] + + +def test_serialize_item_level_properties_single_select(): + """Test serialization of single select property""" + # Setup + client = Mock() + dataset = Mock(team="test_team") + + property_value_id = "123" + property_id = "456" + + # Mock property value + property_value = PropertyValue(id=property_value_id, value="option1") + + # Mock full property + full_property = FullProperty( + id=property_id, + name="test_property", + type="single_select", + property_values=[property_value], + required=False, + granularity=PropertyGranularity("item"), + ) + + # Mock team properties lookup + mock_lookup_response = ({}, {"test_property": full_property}) + + with patch( + "darwin.importer.importer._get_team_properties_annotation_lookup", + return_value=mock_lookup_response, + ): + result = _serialize_item_level_properties( + [{"name": "test_property", "value": "option1"}], + client, + dataset, + False, + False, + ) + + expected = [ + {"actors": [], "property_id": property_id, "value": {"id": property_value_id}} + ] + + assert result == expected + + +def test_serialize_item_level_properties_text(): + """Test serialization of text property""" + # Setup + client = Mock() + dataset = Mock(team="test_team") + + property_id = "789" + + # Mock full property + full_property = FullProperty( + id=property_id, + name="text_property", + type="text", + required=False, + granularity=PropertyGranularity("item"), + ) + + # Mock team properties lookup + mock_lookup_response = ({}, {"text_property": full_property}) + + with patch( + "darwin.importer.importer._get_team_properties_annotation_lookup", + return_value=mock_lookup_response, + ): + result = _serialize_item_level_properties( + [{"name": "text_property", "value": "some text"}], + client, + dataset, + False, + False, + ) + + expected = [ + {"actors": [], "property_id": property_id, "value": {"text": "some text"}} + ] + + assert result == expected + + +def test_serialize_item_level_properties_with_actors(): + """Test serialization with annotators and reviewers""" + # Setup + client = Mock() + dataset = Mock(team="test_team") + + property_id = "789" + + # Mock full property + full_property = FullProperty( + id=property_id, + name="text_property", + type="text", + required=False, + granularity=PropertyGranularity("item"), + ) + + # Mock team properties lookup + mock_lookup_response = ({}, {"text_property": full_property}) + + # Mock annotator and reviewer handlers + mock_annotator = {"email": "annotator@test.com", "role": "annotator"} + mock_reviewer = {"email": "reviewer@test.com", "role": "reviewer"} + + with patch( + "darwin.importer.importer._get_team_properties_annotation_lookup", + return_value=mock_lookup_response, + ), patch( + "darwin.importer.importer._handle_annotators", return_value=[mock_annotator] + ), patch( + "darwin.importer.importer._handle_reviewers", return_value=[mock_reviewer] + ): + result = _serialize_item_level_properties( + [{"name": "text_property", "value": "some text"}], + client, + dataset, + True, + True, + ) + + expected = [ + { + "actors": [mock_annotator, mock_reviewer], + "property_id": property_id, + "value": {"text": "some text"}, + } + ] + + assert result == expected + + +def test_serialize_item_level_properties_multiple_properties(): + """Test serialization of multiple properties""" + # Setup + client = Mock() + dataset = Mock(team="test_team") + + # Mock properties + text_property = FullProperty( + id="123", + name="text_property", + type="text", + required=False, + granularity=PropertyGranularity("item"), + ) + + select_property_value = PropertyValue(id="456", value="option1") + select_property = FullProperty( + id="789", + name="select_property", + type="single_select", + property_values=[select_property_value], + required=False, + granularity=PropertyGranularity("item"), + ) + + # Mock team properties lookup + mock_lookup_response = ( + {}, + {"text_property": text_property, "select_property": select_property}, + ) + + with patch( + "darwin.importer.importer._get_team_properties_annotation_lookup", + return_value=mock_lookup_response, + ): + result = _serialize_item_level_properties( + [ + {"name": "text_property", "value": "some text"}, + {"name": "select_property", "value": "option1"}, + ], + client, + dataset, + False, + False, + ) + + expected = [ + {"actors": [], "property_id": "123", "value": {"text": "some text"}}, + {"actors": [], "property_id": "789", "value": {"id": "456"}}, + ] + + assert result == expected