Skip to content

Commit

Permalink
[DAR-5929] Allow annotation and text properties import (#1009)
Browse files Browse the repository at this point in the history 
* [DAR-5929] Allow annotation and text properties import

* [DAR-5929] Revert change of base url

* [DAR-5929] Add additional tests

* [DAR-5929] Support empty values for text properties and test same name for item and section/annotation level props

* [DAR-5929] Fix linter

* [DAR-5929] Remove accidental import

* [DAR-5929] Add support for null value text properties

* [DAR-5929] Remove unnecessary check for empty string

* [DAR-5929] Support null values for text item level props

* [DAR-5929] Revert change of base url

* [DAR-5929] Format files

* [DAR-5929] Support null values for multi-select and single-select

---------

Co-authored-by: Umberto <[email protected]>
  • Loading branch information
@aleksandar-ivic @umbertoDifa
aleksandar-ivic and umbertoDifa authored Feb 28, 2025
1 parent 63b2aed commit 67daf74
Show file tree
Hide file tree
Showing 18 changed files with 304 additions and 74 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ test_output_*
!darwin/future/tests/data
# scripts
test.py
run.py

# pyenv
.python-version
Expand Down
168 changes: 103 additions & 65 deletions darwin/importer/importer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,11 @@ def _serialize_item_level_properties(
_, team_item_properties_lookup = _get_team_properties_annotation_lookup(
client, dataset.team
)
# We will skip text item properties that have value null
for item_property_value in item_property_values:
item_property = team_item_properties_lookup[item_property_value["name"]]
item_property_id = item_property.id
value = None
if (
item_property.type == "single_select"
or item_property.type == "multi_select"
Expand All @@ -472,25 +474,30 @@ def _serialize_item_level_properties(
),
None,
)
value = {"id": item_property_value_id}
elif item_property.type == "text":
if item_property_value_id is not None:
value = {"id": item_property_value_id}
elif item_property.type == "text" and item_property_value["value"] is not None:
value = {"text": item_property_value["value"]}
actors: List[dt.DictFreeForm] = []
actors.extend(
_handle_annotators(
import_annotators, item_property_value=item_property_value
if value is not None:
actors: List[dt.DictFreeForm] = []
actors.extend(
_handle_annotators(
import_annotators, item_property_value=item_property_value
)
)

actors.extend(
_handle_reviewers(
import_reviewers, item_property_value=item_property_value
)
)
serialized_item_level_properties.append(
{
"actors": actors,
"property_id": item_property_id,
"value": value,
}
)
)
actors.extend(
_handle_reviewers(import_reviewers, item_property_value=item_property_value)
)
serialized_item_level_properties.append(
{
"actors": actors,
"property_id": item_property_id,
"value": value,
}
)

return serialized_item_level_properties

Expand Down Expand Up @@ -623,6 +630,14 @@ def _import_properties(
t_prop: FullProperty = team_properties_annotation_lookup[
(a_prop.name, annotation_class_id)
]
if t_prop.type == "text":
set_text_property_value(
annotation_property_map,
annotation_id,
a_prop,
t_prop,
)
continue

# if property value is None, update annotation_property_map with empty set
if a_prop.value is None:
Expand Down Expand Up @@ -759,14 +774,15 @@ def _import_properties(
continue

# check if property value is different in m_prop (.v7/metadata.json) options
for m_prop_option in m_prop_options:
if m_prop_option.get("value") == a_prop.value:
break
else:
if a_prop.value:
raise ValueError(
f"Annotation: '{annotation_name}' -> Property '{a_prop.value}' not found in .v7/metadata.json, found: {m_prop.property_values}"
)
if m_prop.type != "text":
for m_prop_option in m_prop_options:
if m_prop_option.get("value") == a_prop.value:
break
else:
if a_prop.value:
raise ValueError(
f"Annotation: '{annotation_name}' -> Property '{a_prop.value}' not found in .v7/metadata.json, found: {m_prop.property_values}"
)

# get team property
t_prop: FullProperty = team_properties_annotation_lookup[
Expand All @@ -782,43 +798,50 @@ def _import_properties(
continue

# check if property value is different in t_prop (team) options
for t_prop_val in t_prop.property_values or []:
if t_prop_val.value == a_prop.value:
break
else:
# if it is, update it
full_property = FullProperty(
id=t_prop.id,
name=a_prop.name,
type=m_prop_type,
required=m_prop.required,
description=m_prop.description
or "property-updated-during-annotation-import",
slug=client.default_team,
annotation_class_id=int(annotation_class_id),
property_values=[
PropertyValue(
value=a_prop.value,
color=m_prop_option.get("color"), # type: ignore
)
],
granularity=t_prop.granularity,
)
# Don't attempt the same propery update multiple times
if (
full_property
not in annotation_and_section_level_properties_to_update
):
annotation_and_section_level_properties_to_update.append(
full_property
if t_prop.type != "text":
for t_prop_val in t_prop.property_values or []:
if t_prop_val.value == a_prop.value:
break
else:
# if it is, update it
full_property = FullProperty(
id=t_prop.id,
name=a_prop.name,
type=m_prop_type,
required=m_prop.required,
description=m_prop.description
or "property-updated-during-annotation-import",
slug=client.default_team,
annotation_class_id=int(annotation_class_id),
property_values=[
PropertyValue(
value=a_prop.value,
color=m_prop_option.get("color"), # type: ignore
)
],
granularity=t_prop.granularity,
)
continue
# Don't attempt the same propery update multiple times
if (
full_property
not in annotation_and_section_level_properties_to_update
):
annotation_and_section_level_properties_to_update.append(
full_property
)
continue

assert t_prop.id is not None
assert t_prop_val.id is not None
annotation_property_map[annotation_id][str(a_prop.frame_index)][
t_prop.id
].add(t_prop_val.id)

if t_prop.type == "text":
set_text_property_value(
annotation_property_map, annotation_id, a_prop, t_prop
)
else:
assert t_prop_val.id is not None
annotation_property_map[annotation_id][str(a_prop.frame_index)][
t_prop.id
].add(t_prop_val.id)

# Create/Update team item properties based on metadata
(
Expand Down Expand Up @@ -998,12 +1021,17 @@ def _import_properties(
] = set()
break

for prop_val in prop.property_values or []:
if prop_val.value == a_prop.value:
annotation_property_map[annotation_id][frame_index][
prop.id
].add(prop_val.id)
break
if prop.type == "text":
set_text_property_value(
annotation_property_map, annotation_id, a_prop, prop
)
else:
for prop_val in prop.property_values or []:
if prop_val.value == a_prop.value:
annotation_property_map[annotation_id][frame_index][
prop.id
].add(prop_val.id)
break
break
_assign_item_properties_to_dataset(
item_properties, team_item_properties_lookup, client, dataset, console
Expand Down Expand Up @@ -2488,3 +2516,13 @@ def slot_is_medical(slot: Dict[str, Any]) -> bool:

def slot_is_handled_by_monai(slot: Dict[str, Any]) -> bool:
return slot.get("metadata", {}).get("medical", {}).get("handler") == "MONAI"


def set_text_property_value(annotation_property_map, annotation_id, a_prop, t_prop):
if a_prop.value is None:
# here we will remove the property value
annotation_property_map[annotation_id][str(a_prop.frame_index)][t_prop.id] = []
else:
annotation_property_map[annotation_id][str(a_prop.frame_index)][t_prop.id] = [
{"text": a_prop.value}
]
3 changes: 2 additions & 1 deletion e2e_tests/cli/test_import.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ def assert_same_item_level_properties(
Ensures that all expected item-level properties are present in exported item-level properties
"""
for expected_item_level_property in expected_item_level_properties:
assert expected_item_level_property in actual_item_level_properties
if expected_item_level_property["value"] is not None:
assert expected_item_level_property in actual_item_level_properties


def compare_annotations_export(
Expand Down
40 changes: 40 additions & 0 deletions e2e_tests/data/import/image_annotations_with_subtypes/image_1.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@
"frame_index": 0,
"name": "single_select-1",
"value": "1"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"slot_names": [
Expand Down Expand Up @@ -93,6 +103,11 @@
"frame_index": 0,
"name": "single_select-1",
"value": "1"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": null
}
],
"slot_names": [
Expand Down Expand Up @@ -122,6 +137,11 @@
"frame_index": 0,
"name": "multi_select-1",
"value": "2"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"slot_names": [
Expand Down Expand Up @@ -158,6 +178,16 @@
"frame_index": 0,
"name": "single_select-1",
"value": "2"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"slot_names": [
Expand Down Expand Up @@ -220,6 +250,11 @@
"frame_index": 0,
"name": "single_select-1",
"value": "2"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
}
],
"slot_names": [
Expand All @@ -245,6 +280,11 @@
"frame_index": 0,
"name": "multi_select-1",
"value": "1"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"skeleton": {
Expand Down
35 changes: 35 additions & 0 deletions e2e_tests/data/import/image_annotations_with_subtypes/image_2.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,16 @@
"frame_index": 0,
"name": "single_select-1",
"value": "1"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"slot_names": [
Expand Down Expand Up @@ -93,6 +103,11 @@
"frame_index": 0,
"name": "single_select-1",
"value": "1"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
}
],
"slot_names": [
Expand Down Expand Up @@ -122,6 +137,11 @@
"frame_index": 0,
"name": "multi_select-1",
"value": "2"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"slot_names": [
Expand Down Expand Up @@ -220,6 +240,16 @@
"frame_index": 0,
"name": "single_select-1",
"value": "2"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
},
{
"frame_index": "global",
"name": "annotation-text-1",
"value": "Annotation test"
}
],
"slot_names": [
Expand Down Expand Up @@ -288,6 +318,11 @@
"frame_index": 0,
"name": "single_select-1",
"value": "1"
},
{
"frame_index": 0,
"name": "section-text-1",
"value": "Section test"
}
],
"slot_names": [
Expand Down
Loading

0 comments on commit 67daf74

Please sign in to comment.