From 92ef9dbb0b6a8ed4581c4ac2c6d126376ed8b683 Mon Sep 17 00:00:00 2001 From: John Wilkie <124276291+JBWilkie@users.noreply.github.com> Date: Fri, 3 May 2024 14:43:13 +0100 Subject: [PATCH] [DAR-1562][External] Allow pulling of unpopulated required property annotations (#825) * Allow SelectedProperty frame_index to be None to allow pull() of non-populated required properties * Black & ruff formatting / linting * Added data to test unpopulated required properties * Added unit test * Improved test --- darwin/exceptions.py | 12 +-- darwin/exporter/formats/yolo_segmented.py | 12 +-- darwin/future/core/items/archive_items.py | 6 +- darwin/future/core/items/assign_items.py | 6 +- darwin/future/core/items/delete_items.py | 6 +- darwin/future/core/items/move_items.py | 6 +- .../future/core/items/move_items_to_folder.py | 6 +- darwin/future/core/items/restore_items.py | 6 +- darwin/future/core/items/set_item_layout.py | 6 +- darwin/future/core/items/set_item_priority.py | 6 +- .../future/core/items/set_stage_to_items.py | 6 +- darwin/future/core/items/tag_items.py | 6 +- darwin/future/core/items/untag_items.py | 6 +- darwin/future/core/types/common.py | 3 +- darwin/future/data_objects/properties.py | 2 +- darwin/future/meta/objects/item.py | 80 ++++++++++++------- .../tests/data_objects/test_properties.py | 9 ++- .../workflow/invalidvaluefortest.py | 3 +- darwin/importer/importer.py | 18 ++--- deploy/confirm_main_branch_deployability.py | 6 +- tests/darwin/importer/importer_test.py | 24 ++---- 21 files changed, 126 insertions(+), 109 deletions(-) diff --git a/darwin/exceptions.py b/darwin/exceptions.py index e42d3621d..681ba7b1e 100644 --- a/darwin/exceptions.py +++ b/darwin/exceptions.py @@ -392,17 +392,13 @@ def __init__( super().__init__("Complex polygons not yet supported for dataloop import") -class ExportException(DarwinException): - ... +class ExportException(DarwinException): ... -class ExportException_CouldNotAssembleOutputPath(ExportException): - ... +class ExportException_CouldNotAssembleOutputPath(ExportException): ... -class ExportException_CouldNotBuildOutput(ExportException): - ... +class ExportException_CouldNotBuildOutput(ExportException): ... -class ExportException_CouldNotWriteFile(ExportException): - ... +class ExportException_CouldNotWriteFile(ExportException): ... diff --git a/darwin/exporter/formats/yolo_segmented.py b/darwin/exporter/formats/yolo_segmented.py index 88702c1c7..40c0c2df7 100644 --- a/darwin/exporter/formats/yolo_segmented.py +++ b/darwin/exporter/formats/yolo_segmented.py @@ -198,11 +198,13 @@ def _handle_polygon( except KeyError as exc: logger.warn( - f"Skipped annotation at index {annotation_index} because an" - "expected key was not found in the data." - f"Error occured while calculating point at index {last_point}." - if last_point - else "Error occured while enumerating points.", + ( + f"Skipped annotation at index {annotation_index} because an" + "expected key was not found in the data." + f"Error occured while calculating point at index {last_point}." + if last_point + else "Error occured while enumerating points." + ), exc_info=exc, ) return False diff --git a/darwin/future/core/items/archive_items.py b/darwin/future/core/items/archive_items.py index b83913b65..2c2123ba3 100644 --- a/darwin/future/core/items/archive_items.py +++ b/darwin/future/core/items/archive_items.py @@ -37,9 +37,9 @@ def archive_list_of_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, } } diff --git a/darwin/future/core/items/assign_items.py b/darwin/future/core/items/assign_items.py index 20520a5ab..c5932efc7 100644 --- a/darwin/future/core/items/assign_items.py +++ b/darwin/future/core/items/assign_items.py @@ -31,9 +31,9 @@ def assign_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "assignee_id": assignee_id, diff --git a/darwin/future/core/items/delete_items.py b/darwin/future/core/items/delete_items.py index 3100b6cc7..7f53c0af4 100644 --- a/darwin/future/core/items/delete_items.py +++ b/darwin/future/core/items/delete_items.py @@ -37,9 +37,9 @@ def delete_list_of_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, } } diff --git a/darwin/future/core/items/move_items.py b/darwin/future/core/items/move_items.py index 08b334b13..b3163fdb2 100644 --- a/darwin/future/core/items/move_items.py +++ b/darwin/future/core/items/move_items.py @@ -44,9 +44,9 @@ def move_items_to_stage( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "stage_id": str(stage_id), diff --git a/darwin/future/core/items/move_items_to_folder.py b/darwin/future/core/items/move_items_to_folder.py index 04296d137..5729626e6 100644 --- a/darwin/future/core/items/move_items_to_folder.py +++ b/darwin/future/core/items/move_items_to_folder.py @@ -40,9 +40,9 @@ def move_list_of_items_to_folder( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "path": path, diff --git a/darwin/future/core/items/restore_items.py b/darwin/future/core/items/restore_items.py index 4aef6282c..2678a9c1a 100644 --- a/darwin/future/core/items/restore_items.py +++ b/darwin/future/core/items/restore_items.py @@ -37,9 +37,9 @@ def restore_list_of_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, } } diff --git a/darwin/future/core/items/set_item_layout.py b/darwin/future/core/items/set_item_layout.py index 014dc9529..43aacb859 100644 --- a/darwin/future/core/items/set_item_layout.py +++ b/darwin/future/core/items/set_item_layout.py @@ -42,9 +42,9 @@ def set_item_layout( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "layout": dict(layout), diff --git a/darwin/future/core/items/set_item_priority.py b/darwin/future/core/items/set_item_priority.py index 888686999..ff9ef63dd 100644 --- a/darwin/future/core/items/set_item_priority.py +++ b/darwin/future/core/items/set_item_priority.py @@ -38,9 +38,9 @@ def set_item_priority( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "priority": priority, diff --git a/darwin/future/core/items/set_stage_to_items.py b/darwin/future/core/items/set_stage_to_items.py index d489c71c5..878d4a750 100644 --- a/darwin/future/core/items/set_stage_to_items.py +++ b/darwin/future/core/items/set_stage_to_items.py @@ -31,9 +31,9 @@ def set_stage_to_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "stage_id": stage_id, diff --git a/darwin/future/core/items/tag_items.py b/darwin/future/core/items/tag_items.py index 2c497a689..5dd79e861 100644 --- a/darwin/future/core/items/tag_items.py +++ b/darwin/future/core/items/tag_items.py @@ -30,9 +30,9 @@ def tag_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "annotation_class_id": tag_id, diff --git a/darwin/future/core/items/untag_items.py b/darwin/future/core/items/untag_items.py index 069966c90..5d0b0acb2 100644 --- a/darwin/future/core/items/untag_items.py +++ b/darwin/future/core/items/untag_items.py @@ -31,9 +31,9 @@ def untag_items( ), "No parameters provided, please provide at least one non-dataset id filter" payload = { "filters": { - "dataset_ids": dataset_ids - if isinstance(dataset_ids, list) - else [dataset_ids], + "dataset_ids": ( + dataset_ids if isinstance(dataset_ids, list) else [dataset_ids] + ), **filters, }, "annotation_class_id": tag_id, diff --git a/darwin/future/core/types/common.py b/darwin/future/core/types/common.py index 0a8eb6e47..8307f7aa7 100644 --- a/darwin/future/core/types/common.py +++ b/darwin/future/core/types/common.py @@ -9,8 +9,7 @@ class Implements_str(Protocol): - def __str__(self) -> str: - ... + def __str__(self) -> str: ... Stringable = Union[str, Implements_str] diff --git a/darwin/future/data_objects/properties.py b/darwin/future/data_objects/properties.py index c3dcb183f..b12ca0a29 100644 --- a/darwin/future/data_objects/properties.py +++ b/darwin/future/data_objects/properties.py @@ -147,7 +147,7 @@ class SelectedProperty(DefaultDarwin): value (str): Value of the property """ - frame_index: int + frame_index: Optional[int] = None name: str type: Optional[str] = None value: Optional[str] = None diff --git a/darwin/future/meta/objects/item.py b/darwin/future/meta/objects/item.py index 3c195c8b4..d402b48be 100644 --- a/darwin/future/meta/objects/item.py +++ b/darwin/future/meta/objects/item.py @@ -60,9 +60,11 @@ class Item(MetaBase[ItemCore]): def delete(self) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) dataset_id = cast(Union[int, List[int]], dataset_id) @@ -72,9 +74,11 @@ def delete(self) -> None: def move_to_folder(self, path: str) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) dataset_id = cast(Union[int, List[int]], dataset_id) @@ -84,9 +88,11 @@ def move_to_folder(self, path: str) -> None: def set_priority(self, priority: int) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) dataset_id = cast(Union[int, List[int]], dataset_id) @@ -96,9 +102,11 @@ def set_priority(self, priority: int) -> None: def restore(self) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) dataset_id = cast(Union[int, List[int]], dataset_id) @@ -108,9 +116,11 @@ def restore(self) -> None: def archive(self) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) dataset_id = cast(Union[int, List[int]], dataset_id) @@ -120,9 +130,11 @@ def archive(self) -> None: def set_layout(self, layout: ItemLayout) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) assert isinstance(layout, ItemLayout) @@ -143,9 +155,11 @@ def assign(self, assignee_id: int, workflow_id: str | None = None) -> None: assert isinstance(workflow_id, str) team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) @@ -158,9 +172,11 @@ def assign(self, assignee_id: int, workflow_id: str | None = None) -> None: def tag(self, tag_id: int) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) if not isinstance(tag_id, int): @@ -172,9 +188,11 @@ def tag(self, tag_id: int) -> None: def untag(self, tag_id: int) -> None: team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) if not isinstance(tag_id, int): @@ -200,9 +218,11 @@ def set_stage( assert isinstance(workflow_id, str) team_slug, dataset_id = ( self.meta_params["team_slug"], - self.meta_params["dataset_id"] - if "dataset_id" in self.meta_params - else self.meta_params["dataset_ids"], + ( + self.meta_params["dataset_id"] + if "dataset_id" in self.meta_params + else self.meta_params["dataset_ids"] + ), ) assert isinstance(team_slug, str) diff --git a/darwin/future/tests/data_objects/test_properties.py b/darwin/future/tests/data_objects/test_properties.py index 36c570c61..6a45917c0 100644 --- a/darwin/future/tests/data_objects/test_properties.py +++ b/darwin/future/tests/data_objects/test_properties.py @@ -2,7 +2,7 @@ import pytest -from darwin.future.data_objects.properties import MetaDataClass +from darwin.future.data_objects.properties import MetaDataClass, SelectedProperty @pytest.fixture @@ -35,3 +35,10 @@ def test_properties_metadata_fails() -> None: path = Path("darwin/future/tests/data/does_not_exist") with pytest.raises(FileNotFoundError): MetaDataClass.from_path(path) + + +def test_can_parse_unpopulated_required_properties() -> None: + selected_property = SelectedProperty( + frame_index=None, name="name", type="type", value=None + ) + assert selected_property is not None diff --git a/darwin/future/tests/data_objects/workflow/invalidvaluefortest.py b/darwin/future/tests/data_objects/workflow/invalidvaluefortest.py index 7dc84b696..47955867a 100644 --- a/darwin/future/tests/data_objects/workflow/invalidvaluefortest.py +++ b/darwin/future/tests/data_objects/workflow/invalidvaluefortest.py @@ -1,2 +1 @@ -class InvalidValueForTest: - ... +class InvalidValueForTest: ... diff --git a/darwin/importer/importer.py b/darwin/importer/importer.py index 785182f74..62a4d2e82 100644 --- a/darwin/importer/importer.py +++ b/darwin/importer/importer.py @@ -256,9 +256,9 @@ def _get_team_properties_annotation_lookup(client, team_slug): team_properties = client.get_team_properties(team_slug) # (property-name, annotation_class_id): FullProperty object - team_properties_annotation_lookup: Dict[ - Tuple[str, Optional[int]], FullProperty - ] = {} + team_properties_annotation_lookup: Dict[Tuple[str, Optional[int]], FullProperty] = ( + {} + ) for prop in team_properties: team_properties_annotation_lookup[(prop.name, prop.annotation_class_id)] = prop @@ -903,9 +903,9 @@ def import_annotations( # noqa: C901 # Need to re parse the files since we didn't save the annotations in memory for local_path in set(local_file.path for local_file in local_files): # noqa: C401 - imported_files: Union[ - List[dt.AnnotationFile], dt.AnnotationFile, None - ] = importer(local_path) + imported_files: Union[List[dt.AnnotationFile], dt.AnnotationFile, None] = ( + importer(local_path) + ) if imported_files is None: parsed_files = [] elif not isinstance(imported_files, List): @@ -1307,9 +1307,9 @@ def _import_annotations( # Insert the default slot name if not available in the import source annotation = _handle_slot_names(annotation, dataset.version, default_slot_name) - annotation_class_ids_map[ - (annotation_class.name, annotation_type) - ] = annotation_class_id + annotation_class_ids_map[(annotation_class.name, annotation_type)] = ( + annotation_class_id + ) serial_obj = { "annotation_class_id": annotation_class_id, "data": data, diff --git a/deploy/confirm_main_branch_deployability.py b/deploy/confirm_main_branch_deployability.py index 53b0f745b..7782c7cdb 100755 --- a/deploy/confirm_main_branch_deployability.py +++ b/deploy/confirm_main_branch_deployability.py @@ -11,8 +11,10 @@ logger = logging.getLogger(__name__) logger.addHandler(logging.StreamHandler(sys.stdout)) -logger.setLevel(logging.DEBUG) if environ.get("DEBUG") else logger.setLevel( - logging.INFO +( + logger.setLevel(logging.DEBUG) + if environ.get("DEBUG") + else logger.setLevel(logging.INFO) ) diff --git a/tests/darwin/importer/importer_test.py b/tests/darwin/importer/importer_test.py index b7822e19c..5e2534a30 100644 --- a/tests/darwin/importer/importer_test.py +++ b/tests/darwin/importer/importer_test.py @@ -23,43 +23,35 @@ def patch_factory(module: str) -> _patch: @pytest.mark.skip("Not yet implemented.") -def test_build_main_annotations_lookup_table() -> None: - ... # TODO: Write this test +def test_build_main_annotations_lookup_table() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") # type: ignore -def test_find_and_parse() -> None: - ... # TODO: Write this test +def test_find_and_parse() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") -def test_build_attribute_lookup() -> None: - ... # TODO: Write this test +def test_build_attribute_lookup() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") -def test_get_remote_files() -> None: - ... # TODO: Write this test +def test_get_remote_files() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") -def test__get_slot_name() -> None: - ... # TODO: Write this test +def test__get_slot_name() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") -def test__resolve_annotation_classes() -> None: - ... # TODO: Write this test +def test__resolve_annotation_classes() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") -def test_import_annotations() -> None: - ... # TODO: Write this test +def test_import_annotations() -> None: ... # TODO: Write this test @pytest.mark.skip("Not yet implemented.") -def test__is_skeleton_class() -> None: - ... # TODO: Write this test +def test__is_skeleton_class() -> None: ... # TODO: Write this test def test__get_skeleton_name() -> None: