From bc431a2e651a4c9b15fbb3c5643cbdc1d9180d74 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Thu, 24 Nov 2022 15:15:32 +0000 Subject: [PATCH 01/21] WIP Dataloop transformer and tests --- darwin/importer/formats/dataloop.py | 7 +- .../importer/formats/dataloop.example.json | 179 ++++++++++++++++++ .../importer/formats/import_dataloop_test.py | 56 ++++++ 3 files changed, 241 insertions(+), 1 deletion(-) create mode 100644 tests/darwin/importer/formats/dataloop.example.json create mode 100644 tests/darwin/importer/formats/import_dataloop_test.py diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index a195f7d59..84b502e4b 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -42,7 +42,7 @@ def _remove_leading_slash(filename: str) -> str: def _parse_annotation(annotation: Dict[str, Any]) -> Optional[dt.Annotation]: annotation_type = annotation["type"] annotation_label = annotation["label"] - if annotation_type not in ["box", "class"]: + if annotation_type not in ["box", "class", "segment"]: raise ValueError(f"Unknown supported annotation type: {annotation_type}") if len(annotation["metadata"]["system"].get("snapshots_", [])) > 1: @@ -58,4 +58,9 @@ def _parse_annotation(annotation: Dict[str, Any]) -> Optional[dt.Annotation]: x2, y2 = coords[1]["x"], coords[1]["y"] return dt.make_bounding_box(annotation_label, x1, y1, x2 - x1, y2 - y1) + if annotation_type == "segment": + coords = annotation["coordinates"] + points: List[dt.Point] = [dt.Point(x=c["x"], y=c["y"]) for c in coords] + return dt.make_polygon(annotation_label, point_path=points) + return None diff --git a/tests/darwin/importer/formats/dataloop.example.json b/tests/darwin/importer/formats/dataloop.example.json new file mode 100644 index 000000000..8973f4caa --- /dev/null +++ b/tests/darwin/importer/formats/dataloop.example.json @@ -0,0 +1,179 @@ +{ + "annotations": [ + { + "id": "1", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "box", + "label": "other", + "attributes": [], + "coordinates": [ + { + "x": 288.81, + "y": 845.49 + }, + { + "x": 2221.32, + "y": 3528.24 + } + ], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + }, + { + "id": "2", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "class", + "label": "other", + "attributes": [], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + }, + { + "id": "3", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "segment", + "label": "other", + "attributes": [], + "coordinates": [ + { + "x": 856.7307692307692, + "y": 1077.8846153846152 + }, + { + "x": 575, + "y": 657.6923076923076 + }, + { + "x": 989.4230769230768, + "y": 409.6153846153846 + }, + { + "x": 974.0384615384614, + "y": 640.3846153846154 + }, + { + "x": 1033.653846153846, + "y": 915.3846153846154 + }, + { + "x": 1106.730769230769, + "y": 1053.8461538461538 + }, + { + "x": 1204.8076923076922, + "y": 1079.8076923076922 + } + ], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + } + ] +} \ No newline at end of file diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py new file mode 100644 index 000000000..4231ad8da --- /dev/null +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -0,0 +1,56 @@ +from os.path import dirname, join +from unittest import TestCase, skip +from unittest.mock import patch + +from darwin.importer.formats.dataloop import ( + _parse_annotation, + _remove_leading_slash, + parse_path, +) + + +class DataLoopTestCase(TestCase): + dataloop_mock_data_fd = open(join(dirname(__file__), "dataloop_mock_data.json")) + dataloop_mock_data = dataloop_mock_data_fd.read() + dataloop_mock_data_fd.close() + del dataloop_mock_data_fd + + +class TestParsePath(DataLoopTestCase): + def test_returns_none_if_file_extension_is_not_json(self): + self.assertIsNone(parse_path("foo.bar")) + + @skip("WIP") + def test_opens_with_list_of_annotations(self): + with patch("darwin.importer.formats.dataloop.pathlib.Path.open") as mock_open: + mock_open.return_value.return_value = self.dataloop_mock_data + + parse_path("foo.json") + mock_open.assert_called_with("foo.json") + # TODO: Continue here + + @skip("WIP") + def test_returns_only_one_of_each_annotation_class(self): + ... + + +class TestRemoveLeadingSlash(DataLoopTestCase): + def test_removes_slash_if_present(self): + self.assertEqual(_remove_leading_slash("/foo"), "foo") + + def test_does_not_remove_slash_if_not_present(self): + self.assertEqual(_remove_leading_slash("foo"), "foo") + + +class TestParseAnnotation(DataLoopTestCase): + @skip("Not yet implemented") + def test_handles_box_type(self): + ... + + @skip("Not yet implemented") + def test_handles_class_type(self): + ... + + @skip("Not yet implemented") + def test_handles_segment_type(self): + ... From 6f0b97d68cfd0775904dac6230599eaf31f0beb4 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Fri, 25 Nov 2022 12:13:52 +0000 Subject: [PATCH 02/21] WIP tests --- darwin/importer/formats/dataloop.py | 1 + .../importer/formats/dataloop.example.json | 179 ------------------ .../importer/formats/import_dataloop_test.py | 60 ++++-- 3 files changed, 43 insertions(+), 197 deletions(-) delete mode 100644 tests/darwin/importer/formats/dataloop.example.json diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index 84b502e4b..5d8963896 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -25,6 +25,7 @@ def parse_path(path: Path) -> Optional[dt.AnnotationFile]: return None with path.open() as f: data = json.load(f) + print("here", data) annotations: List[dt.Annotation] = list(filter(None, map(_parse_annotation, data["annotations"]))) annotation_classes: Set[dt.AnnotationClass] = set([annotation.annotation_class for annotation in annotations]) return dt.AnnotationFile( diff --git a/tests/darwin/importer/formats/dataloop.example.json b/tests/darwin/importer/formats/dataloop.example.json deleted file mode 100644 index 8973f4caa..000000000 --- a/tests/darwin/importer/formats/dataloop.example.json +++ /dev/null @@ -1,179 +0,0 @@ -{ - "annotations": [ - { - "id": "1", - "datasetId": "61d5e83f5575066ffd58fcda", - "type": "box", - "label": "other", - "attributes": [], - "coordinates": [ - { - "x": 288.81, - "y": 845.49 - }, - { - "x": 2221.32, - "y": 3528.24 - } - ], - "metadata": { - "system": { - "status": null, - "startTime": 0, - "endTime": 1, - "frame": 0, - "endFrame": 1, - "snapshots_": [], - "parentId": null, - "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", - "automated": false, - "objectId": "2", - "isOpen": false, - "isOnlyLocal": false, - "attributes": { - "1": false - }, - "clientParentId": null, - "system": false, - "description": null, - "itemLinks": [], - "openAnnotationVersion": "1.29.1-rc.8", - "recipeId": "61d5e83f71a8721c42b8f02e", - "taskId": "61d5eb2f3e1b9e55742d498c", - "assignmentId": "61d5eb2f3e1b9eb7c22d498d" - }, - "user": {} - }, - "creator": "vakasix267@zoeyy.com", - "createdAt": "2022-01-05T19:26:23.252Z", - "updatedBy": "vakasix267@zoeyy.com", - "updatedAt": "2022-01-05T19:26:23.252Z", - "itemId": "61d5eb9d14b5b8ea83e18037", - "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", - "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", - "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", - "hash": "e8726832771219919976179815544bba59bf15d5", - "source": "ui" - }, - { - "id": "2", - "datasetId": "61d5e83f5575066ffd58fcda", - "type": "class", - "label": "other", - "attributes": [], - "metadata": { - "system": { - "status": null, - "startTime": 0, - "endTime": 1, - "frame": 0, - "endFrame": 1, - "snapshots_": [], - "parentId": null, - "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", - "automated": false, - "objectId": "2", - "isOpen": false, - "isOnlyLocal": false, - "attributes": { - "1": false - }, - "clientParentId": null, - "system": false, - "description": null, - "itemLinks": [], - "openAnnotationVersion": "1.29.1-rc.8", - "recipeId": "61d5e83f71a8721c42b8f02e", - "taskId": "61d5eb2f3e1b9e55742d498c", - "assignmentId": "61d5eb2f3e1b9eb7c22d498d" - }, - "user": {} - }, - "creator": "vakasix267@zoeyy.com", - "createdAt": "2022-01-05T19:26:23.252Z", - "updatedBy": "vakasix267@zoeyy.com", - "updatedAt": "2022-01-05T19:26:23.252Z", - "itemId": "61d5eb9d14b5b8ea83e18037", - "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", - "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", - "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", - "hash": "e8726832771219919976179815544bba59bf15d5", - "source": "ui" - }, - { - "id": "3", - "datasetId": "61d5e83f5575066ffd58fcda", - "type": "segment", - "label": "other", - "attributes": [], - "coordinates": [ - { - "x": 856.7307692307692, - "y": 1077.8846153846152 - }, - { - "x": 575, - "y": 657.6923076923076 - }, - { - "x": 989.4230769230768, - "y": 409.6153846153846 - }, - { - "x": 974.0384615384614, - "y": 640.3846153846154 - }, - { - "x": 1033.653846153846, - "y": 915.3846153846154 - }, - { - "x": 1106.730769230769, - "y": 1053.8461538461538 - }, - { - "x": 1204.8076923076922, - "y": 1079.8076923076922 - } - ], - "metadata": { - "system": { - "status": null, - "startTime": 0, - "endTime": 1, - "frame": 0, - "endFrame": 1, - "snapshots_": [], - "parentId": null, - "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", - "automated": false, - "objectId": "2", - "isOpen": false, - "isOnlyLocal": false, - "attributes": { - "1": false - }, - "clientParentId": null, - "system": false, - "description": null, - "itemLinks": [], - "openAnnotationVersion": "1.29.1-rc.8", - "recipeId": "61d5e83f71a8721c42b8f02e", - "taskId": "61d5eb2f3e1b9e55742d498c", - "assignmentId": "61d5eb2f3e1b9eb7c22d498d" - }, - "user": {} - }, - "creator": "vakasix267@zoeyy.com", - "createdAt": "2022-01-05T19:26:23.252Z", - "updatedBy": "vakasix267@zoeyy.com", - "updatedAt": "2022-01-05T19:26:23.252Z", - "itemId": "61d5eb9d14b5b8ea83e18037", - "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", - "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", - "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", - "hash": "e8726832771219919976179815544bba59bf15d5", - "source": "ui" - } - ] -} \ No newline at end of file diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 4231ad8da..8da051795 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -1,6 +1,8 @@ -from os.path import dirname, join +from os.path import dirname, join, realpath +from pathlib import Path +from typing import Dict from unittest import TestCase, skip -from unittest.mock import patch +from unittest.mock import MagicMock, Mock, patch from darwin.importer.formats.dataloop import ( _parse_annotation, @@ -10,28 +12,50 @@ class DataLoopTestCase(TestCase): - dataloop_mock_data_fd = open(join(dirname(__file__), "dataloop_mock_data.json")) - dataloop_mock_data = dataloop_mock_data_fd.read() - dataloop_mock_data_fd.close() - del dataloop_mock_data_fd + def setUp(self): + _fd = open(realpath(join(dirname(__file__), "..", "..", "data", "dataloop.example.json"))) + self.DATALOOP_MOCK_DATA = _fd.read() + _fd.close() + + DARWIN_PARSED_DATA = { + "filename": "test.jpg", + "annotations": [ + {"class": "class_1"}, + {"class": "class_2"}, + {"class": "class_3"}, + ], + } class TestParsePath(DataLoopTestCase): - def test_returns_none_if_file_extension_is_not_json(self): - self.assertIsNone(parse_path("foo.bar")) + @patch( + "darwin.importer.formats.dataloop._remove_leading_slash", + ) + def test_returns_none_if_file_extension_is_not_json(self, mock_remove_leading_slash): + self.assertIsNone(parse_path(Path("foo.bar"))) - @skip("WIP") - def test_opens_with_list_of_annotations(self): - with patch("darwin.importer.formats.dataloop.pathlib.Path.open") as mock_open: - mock_open.return_value.return_value = self.dataloop_mock_data + @patch( + "darwin.importer.formats.dataloop._remove_leading_slash", + ) + @patch("darwin.importer.formats.dataloop.json.load") + @patch("darwin.importer.formats.dataloop.Path.open") + @patch("darwin.importer.formats.dataloop._parse_annotation") + def test_opens_annotations_file_and_parses( + self, + _parse_annotation_mock: MagicMock, + path_open_mock: MagicMock, + json_load_mock: MagicMock, + mock_remove_leading_slash: MagicMock, + ): + json_load_mock.return_value = self.DARWIN_PARSED_DATA + test_path = "foo.json" - parse_path("foo.json") - mock_open.assert_called_with("foo.json") - # TODO: Continue here + parse_path(Path(test_path)) - @skip("WIP") - def test_returns_only_one_of_each_annotation_class(self): - ... + _parse_annotation_mock.assert_called_once() + path_open_mock.assert_called_once() + json_load_mock.assert_called_once() + mock_remove_leading_slash.assert_called_once() class TestRemoveLeadingSlash(DataLoopTestCase): From 959fa7dbd9618ea7234671d72ee8408af4540651 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Fri, 25 Nov 2022 16:19:36 +0000 Subject: [PATCH 03/21] WIP: issue still in place with pycache --- .python-version | 1 + .../importer/formats/import_dataloop_test.py | 39 +++++++++++++++---- 2 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 .python-version diff --git a/.python-version b/.python-version new file mode 100644 index 000000000..bd28b9c5c --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.9 diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 8da051795..b5509a4db 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -1,9 +1,12 @@ +from json import loads as json_loads from os.path import dirname, join, realpath from pathlib import Path +from pprint import pprint from typing import Dict from unittest import TestCase, skip from unittest.mock import MagicMock, Mock, patch +from darwin.datatypes import Annotation from darwin.importer.formats.dataloop import ( _parse_annotation, _remove_leading_slash, @@ -28,6 +31,9 @@ def setUp(self): class TestParsePath(DataLoopTestCase): + def tearDown(self): + patch.stopall() + @patch( "darwin.importer.formats.dataloop._remove_leading_slash", ) @@ -52,13 +58,16 @@ def test_opens_annotations_file_and_parses( parse_path(Path(test_path)) - _parse_annotation_mock.assert_called_once() + self.assertEqual(_parse_annotation_mock.call_count, 3) path_open_mock.assert_called_once() json_load_mock.assert_called_once() mock_remove_leading_slash.assert_called_once() class TestRemoveLeadingSlash(DataLoopTestCase): + def tearDown(self) -> None: + patch.stopall() + def test_removes_slash_if_present(self): self.assertEqual(_remove_leading_slash("/foo"), "foo") @@ -67,14 +76,30 @@ def test_does_not_remove_slash_if_not_present(self): class TestParseAnnotation(DataLoopTestCase): - @skip("Not yet implemented") + def setUp(self): + super().setUp() + self.parsed_json = json_loads(self.DATALOOP_MOCK_DATA) + + def tearDown(self) -> None: + patch.stopall() + def test_handles_box_type(self): - ... + from darwin.importer.formats.dataloop import _parse_annotation as pa + + with patch("darwin.importer.formats.dataloop.dt.make_bounding_box") as make_bounding_box_mock: + make_bounding_box_mock.return_value = Annotation("class_1", 0, 0, 0, 0) + pa(self.parsed_json["annotations"][0]) # 0 is a box type + + make_bounding_box_mock.assert_called_with("box_class", 288.81, 845.49, 1932.5100000000002, 2682.75) - @skip("Not yet implemented") def test_handles_class_type(self): - ... + annotation = _parse_annotation(self.parsed_json["annotations"][1]) # 1 is a class type + self.assertEqual(annotation, None) - @skip("Not yet implemented") def test_handles_segment_type(self): - ... + from darwin.importer.formats.dataloop import _parse_annotation as pa + + with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: + pa(self.parsed_json["annotations"][2]) # 2 is a segment type + + make_polygon_mock.assert_called_with("segment_class", 288.81, 845.49, 1932.5100000000002, 2682.75) From 054cb52f36f9fdb7756cdcf3e556188f701c0956 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 11:50:39 +0000 Subject: [PATCH 04/21] All tests now passing. --- darwin/importer/formats/dataloop.py | 2 +- .../importer/formats/import_dataloop_test.py | 30 +++++++++++++++---- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index 5d8963896..c25365790 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -61,7 +61,7 @@ def _parse_annotation(annotation: Dict[str, Any]) -> Optional[dt.Annotation]: if annotation_type == "segment": coords = annotation["coordinates"] - points: List[dt.Point] = [dt.Point(x=c["x"], y=c["y"]) for c in coords] + points: List[dt.Point] = [{"x": c["x"], "y": c["y"]} for c in coords] return dt.make_polygon(annotation_label, point_path=points) return None diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index b5509a4db..9b4f88302 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -1,10 +1,10 @@ from json import loads as json_loads +from math import isclose as math_isclose from os.path import dirname, join, realpath from pathlib import Path -from pprint import pprint -from typing import Dict -from unittest import TestCase, skip -from unittest.mock import MagicMock, Mock, patch +from typing import Union +from unittest import TestCase +from unittest.mock import MagicMock, patch from darwin.datatypes import Annotation from darwin.importer.formats.dataloop import ( @@ -15,11 +15,14 @@ class DataLoopTestCase(TestCase): - def setUp(self): + def setUp(self) -> None: _fd = open(realpath(join(dirname(__file__), "..", "..", "data", "dataloop.example.json"))) self.DATALOOP_MOCK_DATA = _fd.read() _fd.close() + def assertApproximatelyEqualNumber(self, a: Union[int, float], b: Union[int, float], places: int = 8): + math_isclose(a, b, rel_tol=10**-places) + DARWIN_PARSED_DATA = { "filename": "test.jpg", "annotations": [ @@ -102,4 +105,19 @@ def test_handles_segment_type(self): with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - make_polygon_mock.assert_called_with("segment_class", 288.81, 845.49, 1932.5100000000002, 2682.75) + point_path = [(p["x"], p["y"]) for p in make_polygon_mock.call_args.kwargs["point_path"]] + expectation_points = [ + (856.73076923, 1077.88461538), + (575, 657.69230769), + (989.42307692, 409.61538462), + (974.03846154, 640.38461538), + (1033.65384615, 915.38461538), + (1106.73076923, 1053.84615385), + (1204.80769231, 1079.80769231), + ] + + [ + self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) + for a, b in zip(point_path, expectation_points) + ] + self.assertTrue(make_polygon_mock.call_args[0][0], "segment_class") From a46de70d2cf9f564732fc5222f4161b76045a81b Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 11:54:35 +0000 Subject: [PATCH 05/21] Linting and formatting --- darwin/importer/formats/dataloop.py | 6 ++- setup.cfg | 2 + .../importer/formats/import_dataloop_test.py | 38 ++++++++++++++----- 3 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 setup.cfg diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index c25365790..008253b3d 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -29,7 +29,11 @@ def parse_path(path: Path) -> Optional[dt.AnnotationFile]: annotations: List[dt.Annotation] = list(filter(None, map(_parse_annotation, data["annotations"]))) annotation_classes: Set[dt.AnnotationClass] = set([annotation.annotation_class for annotation in annotations]) return dt.AnnotationFile( - path, _remove_leading_slash(data["filename"]), annotation_classes, annotations, remote_path="/" + path, + _remove_leading_slash(data["filename"]), + annotation_classes, + annotations, + remote_path="/", ) diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 000000000..905c8bbe4 --- /dev/null +++ b/setup.cfg @@ -0,0 +1,2 @@ +[flake8] +max-line-length = 160 \ No newline at end of file diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 9b4f88302..47fbb9492 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -16,11 +16,17 @@ class DataLoopTestCase(TestCase): def setUp(self) -> None: - _fd = open(realpath(join(dirname(__file__), "..", "..", "data", "dataloop.example.json"))) + _fd = open( + realpath( + join(dirname(__file__), "..", "..", "data", "dataloop.example.json") + ) + ) self.DATALOOP_MOCK_DATA = _fd.read() _fd.close() - def assertApproximatelyEqualNumber(self, a: Union[int, float], b: Union[int, float], places: int = 8): + def assertApproximatelyEqualNumber( + self, a: Union[int, float], b: Union[int, float], places: int = 8 + ): math_isclose(a, b, rel_tol=10**-places) DARWIN_PARSED_DATA = { @@ -40,7 +46,9 @@ def tearDown(self): @patch( "darwin.importer.formats.dataloop._remove_leading_slash", ) - def test_returns_none_if_file_extension_is_not_json(self, mock_remove_leading_slash): + def test_returns_none_if_file_extension_is_not_json( + self, mock_remove_leading_slash + ): self.assertIsNone(parse_path(Path("foo.bar"))) @patch( @@ -89,23 +97,34 @@ def tearDown(self) -> None: def test_handles_box_type(self): from darwin.importer.formats.dataloop import _parse_annotation as pa - with patch("darwin.importer.formats.dataloop.dt.make_bounding_box") as make_bounding_box_mock: + with patch( + "darwin.importer.formats.dataloop.dt.make_bounding_box" + ) as make_bounding_box_mock: make_bounding_box_mock.return_value = Annotation("class_1", 0, 0, 0, 0) pa(self.parsed_json["annotations"][0]) # 0 is a box type - make_bounding_box_mock.assert_called_with("box_class", 288.81, 845.49, 1932.5100000000002, 2682.75) + make_bounding_box_mock.assert_called_with( + "box_class", 288.81, 845.49, 1932.5100000000002, 2682.75 + ) def test_handles_class_type(self): - annotation = _parse_annotation(self.parsed_json["annotations"][1]) # 1 is a class type + annotation = _parse_annotation( + self.parsed_json["annotations"][1] + ) # 1 is a class type self.assertEqual(annotation, None) def test_handles_segment_type(self): from darwin.importer.formats.dataloop import _parse_annotation as pa - with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: + with patch( + "darwin.importer.formats.dataloop.dt.make_polygon" + ) as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - point_path = [(p["x"], p["y"]) for p in make_polygon_mock.call_args.kwargs["point_path"]] + point_path = [ + (p["x"], p["y"]) + for p in make_polygon_mock.call_args.kwargs["point_path"] + ] expectation_points = [ (856.73076923, 1077.88461538), (575, 657.69230769), @@ -117,7 +136,8 @@ def test_handles_segment_type(self): ] [ - self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) + self.assertApproximatelyEqualNumber(a[0], b[0]) + and self.assertApproximatelyEqualNumber(a[1], b[1]) for a, b in zip(point_path, expectation_points) ] self.assertTrue(make_polygon_mock.call_args[0][0], "segment_class") From fd0d950f41c657728c6715cd337ed447424fb63e Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 12:46:38 +0000 Subject: [PATCH 06/21] Added dev tools to setup.py to allow "pip install 'darwin-py[dev]'" --- setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index d7601ca7f..d61f1d3c2 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ with open(Path(__file__).parent / "darwin" / "version" / "__init__.py", "r") as f: content = f.read() # from https://www.py4u.net/discuss/139845 - version = re.search(r'__version__\s*=\s*[\'"]([^\'"]*)[\'"]', content).group(1) + version = re.search(r'__version__\s*=\s*[\'"]([^\'"]*)[\'"]', content).group(1) # type: ignore with open("README.md", "rb") as f: long_description = f.read().decode("utf-8") @@ -37,6 +37,7 @@ ], extras_require={ "test": ["responses", "pytest", "pytest-describe", "scikit-learn"], + "dev": ["black", "flake8", "isort", "mypy", "responses", "pytest", "pytest-describe", "scikit-learn"], "ml": ["scikit-learn", "torch", "torchvision"], "medical": ["nibabel", "connected-components-3d"], }, From ca04f4c56ef5dc163823e15a02e1b542506c96c9 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 12:50:24 +0000 Subject: [PATCH 07/21] Remove print statement --- darwin/importer/formats/dataloop.py | 1 - 1 file changed, 1 deletion(-) diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index 008253b3d..dc94c5011 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -25,7 +25,6 @@ def parse_path(path: Path) -> Optional[dt.AnnotationFile]: return None with path.open() as f: data = json.load(f) - print("here", data) annotations: List[dt.Annotation] = list(filter(None, map(_parse_annotation, data["annotations"]))) annotation_classes: Set[dt.AnnotationClass] = set([annotation.annotation_class for annotation in annotations]) return dt.AnnotationFile( From bf79de7dae31218b48f4780086a3c0276523a974 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 13:08:21 +0000 Subject: [PATCH 08/21] Added specific, bubbling test, that can be caught at CLI --- darwin/exceptions.py | 19 ++++++++ darwin/importer/formats/dataloop.py | 3 +- .../importer/formats/import_dataloop_test.py | 48 ++++++++----------- 3 files changed, 40 insertions(+), 30 deletions(-) diff --git a/darwin/exceptions.py b/darwin/exceptions.py index f86319d3e..a77aec1c1 100644 --- a/darwin/exceptions.py +++ b/darwin/exceptions.py @@ -142,3 +142,22 @@ def __init__(self, version: str): def __str__(self): return f"Unknown version: '{self.version}'" + + +class UnsupportedImportAnnotationType(Exception): + """ + Used when one tries to parse an annotation with an unsupported type. + """ + + def __init__(self, import_type: str, annotation_type: str): + """ + Parameters + ---------- + import_type: str + The type of import, e.g. "dataloop". + annotation_type: str + The unsupported annotation type. + """ + super().__init__(f"Unsupported annotation type {annotation_type} for {import_type} import") + self.import_type = import_type + self.annotation_type = annotation_type diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index dc94c5011..c24243a07 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -3,6 +3,7 @@ from typing import Any, Dict, List, Optional, Set import darwin.datatypes as dt +from darwin.exceptions import UnsupportedImportAnnotationType def parse_path(path: Path) -> Optional[dt.AnnotationFile]: @@ -47,7 +48,7 @@ def _parse_annotation(annotation: Dict[str, Any]) -> Optional[dt.Annotation]: annotation_type = annotation["type"] annotation_label = annotation["label"] if annotation_type not in ["box", "class", "segment"]: - raise ValueError(f"Unknown supported annotation type: {annotation_type}") + raise UnsupportedImportAnnotationType("dataloop", annotation_type) if len(annotation["metadata"]["system"].get("snapshots_", [])) > 1: raise ValueError("multiple snapshots per annotations are not supported") diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 47fbb9492..19fce658b 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock, patch from darwin.datatypes import Annotation +from darwin.exceptions import UnsupportedImportAnnotationType from darwin.importer.formats.dataloop import ( _parse_annotation, _remove_leading_slash, @@ -16,17 +17,11 @@ class DataLoopTestCase(TestCase): def setUp(self) -> None: - _fd = open( - realpath( - join(dirname(__file__), "..", "..", "data", "dataloop.example.json") - ) - ) + _fd = open(realpath(join(dirname(__file__), "..", "..", "data", "dataloop.example.json"))) self.DATALOOP_MOCK_DATA = _fd.read() _fd.close() - def assertApproximatelyEqualNumber( - self, a: Union[int, float], b: Union[int, float], places: int = 8 - ): + def assertApproximatelyEqualNumber(self, a: Union[int, float], b: Union[int, float], places: int = 8): math_isclose(a, b, rel_tol=10**-places) DARWIN_PARSED_DATA = { @@ -46,9 +41,7 @@ def tearDown(self): @patch( "darwin.importer.formats.dataloop._remove_leading_slash", ) - def test_returns_none_if_file_extension_is_not_json( - self, mock_remove_leading_slash - ): + def test_returns_none_if_file_extension_is_not_json(self, mock_remove_leading_slash): self.assertIsNone(parse_path(Path("foo.bar"))) @patch( @@ -97,34 +90,23 @@ def tearDown(self) -> None: def test_handles_box_type(self): from darwin.importer.formats.dataloop import _parse_annotation as pa - with patch( - "darwin.importer.formats.dataloop.dt.make_bounding_box" - ) as make_bounding_box_mock: + with patch("darwin.importer.formats.dataloop.dt.make_bounding_box") as make_bounding_box_mock: make_bounding_box_mock.return_value = Annotation("class_1", 0, 0, 0, 0) pa(self.parsed_json["annotations"][0]) # 0 is a box type - make_bounding_box_mock.assert_called_with( - "box_class", 288.81, 845.49, 1932.5100000000002, 2682.75 - ) + make_bounding_box_mock.assert_called_with("box_class", 288.81, 845.49, 1932.5100000000002, 2682.75) def test_handles_class_type(self): - annotation = _parse_annotation( - self.parsed_json["annotations"][1] - ) # 1 is a class type + annotation = _parse_annotation(self.parsed_json["annotations"][1]) # 1 is a class type self.assertEqual(annotation, None) def test_handles_segment_type(self): from darwin.importer.formats.dataloop import _parse_annotation as pa - with patch( - "darwin.importer.formats.dataloop.dt.make_polygon" - ) as make_polygon_mock: + with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - point_path = [ - (p["x"], p["y"]) - for p in make_polygon_mock.call_args.kwargs["point_path"] - ] + point_path = [(p["x"], p["y"]) for p in make_polygon_mock.call_args.kwargs["point_path"]] expectation_points = [ (856.73076923, 1077.88461538), (575, 657.69230769), @@ -136,8 +118,16 @@ def test_handles_segment_type(self): ] [ - self.assertApproximatelyEqualNumber(a[0], b[0]) - and self.assertApproximatelyEqualNumber(a[1], b[1]) + self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) for a, b in zip(point_path, expectation_points) ] self.assertTrue(make_polygon_mock.call_args[0][0], "segment_class") + + def test_throws_on_unknown_type(self): + try: + _parse_annotation(self.parsed_json["annotations"][3]) # 3 is an unsupported type + except UnsupportedImportAnnotationType as e: + self.assertEqual(e.import_type, "dataloop") + self.assertEqual(e.annotation_type, "UNSUPPORTED_TYPE") + except Exception as e: + self.fail(f"Test threw wrong exception: {e}") From 5ba66b7a2851f5c143b821dd727672139ad7fdc4 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 13:22:13 +0000 Subject: [PATCH 09/21] Unexpected error catcher for CLI --- darwin/cli.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/darwin/cli.py b/darwin/cli.py index d307904a1..051793945 100644 --- a/darwin/cli.py +++ b/darwin/cli.py @@ -2,7 +2,10 @@ import getpass import os +import platform from argparse import ArgumentParser, Namespace +from datetime import datetime +from json import dumps import requests.exceptions @@ -40,6 +43,21 @@ def main() -> None: f._error("The team specified is not in the configuration, please authenticate first.") except requests.exceptions.ConnectionError: f._error("Darwin seems unreachable, please try again in a minute or contact support.") + except Exception as e: # Catch unhandled exceptions + filename = f"darwin_error_{datetime.now().timestamp()}.log" + + fd = open(filename, "w") + fd.write("Darwin CLI error log") + fd.write(f"Version: {__version__}") + fd.write(f"OS: {platform.platform()}") + fd.write(f"Command: {dumps(args, check_circular=True)}") + fd.write(f"Error: {dumps(e, check_circular=True)}") + fd.close() + + f._error( + "An unexpected error occurred, errors have been written to {filename}, please contact support, and send them the file." + + str(e) + ) def _run(args: Namespace, parser: ArgumentParser) -> None: From 668f0a230d720b28979231bd0346a93a2a6af09a Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 15:01:44 +0000 Subject: [PATCH 10/21] Added the dataloop example after removing it from the gitignore --- .gitignore | 1 + tests/darwin/data/dataloop.example.json | 254 ++++++++++++++++++++++++ 2 files changed, 255 insertions(+) create mode 100644 tests/darwin/data/dataloop.example.json diff --git a/.gitignore b/.gitignore index 7c80aea09..29622440c 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ __pycache__/ output/ data/ +!tests/darwin/data darwin_py.egg-info/PKG-INFO *.png diff --git a/tests/darwin/data/dataloop.example.json b/tests/darwin/data/dataloop.example.json new file mode 100644 index 000000000..f95d80e6b --- /dev/null +++ b/tests/darwin/data/dataloop.example.json @@ -0,0 +1,254 @@ +{ + "annotations": [ + { + "id": "1", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "box", + "label": "box_class", + "attributes": [], + "coordinates": [ + { + "x": 288.81, + "y": 845.49 + }, + { + "x": 2221.32, + "y": 3528.24 + } + ], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + }, + { + "id": "2", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "class", + "label": "class_class", + "attributes": [], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + }, + { + "id": "3", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "segment", + "label": "segment_class", + "attributes": [], + "coordinates": [ + { + "x": 856.7307692307692, + "y": 1077.8846153846152 + }, + { + "x": 575, + "y": 657.6923076923076 + }, + { + "x": 989.4230769230768, + "y": 409.6153846153846 + }, + { + "x": 974.0384615384614, + "y": 640.3846153846154 + }, + { + "x": 1033.653846153846, + "y": 915.3846153846154 + }, + { + "x": 1106.730769230769, + "y": 1053.8461538461538 + }, + { + "x": 1204.8076923076922, + "y": 1079.8076923076922 + } + ], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + }, + { + "id": "3", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "UNSUPPORTED_TYPE", + "label": "segment_class", + "attributes": [], + "coordinates": [ + { + "x": 856.7307692307692, + "y": 1077.8846153846152 + }, + { + "x": 575, + "y": 657.6923076923076 + }, + { + "x": 989.4230769230768, + "y": 409.6153846153846 + }, + { + "x": 974.0384615384614, + "y": 640.3846153846154 + }, + { + "x": 1033.653846153846, + "y": 915.3846153846154 + }, + { + "x": 1106.730769230769, + "y": 1053.8461538461538 + }, + { + "x": 1204.8076923076922, + "y": 1079.8076923076922 + } + ], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" + } + ] +} \ No newline at end of file From 1b9188df48e513732c3f52543de62de91b4266c9 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 15:49:55 +0000 Subject: [PATCH 11/21] Removing python version because I think it's breaking the build --- .python-version | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .python-version diff --git a/.python-version b/.python-version deleted file mode 100644 index bd28b9c5c..000000000 --- a/.python-version +++ /dev/null @@ -1 +0,0 @@ -3.9 From 6970c0ed90cea550d3ff8d897d877f59403f40cf Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 16:28:01 +0000 Subject: [PATCH 12/21] Trying to get 3.6 JIT to understand --- tests/darwin/importer/formats/import_dataloop_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 19fce658b..9103cf01c 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -106,7 +106,7 @@ def test_handles_segment_type(self): with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - point_path = [(p["x"], p["y"]) for p in make_polygon_mock.call_args.kwargs["point_path"]] + point_path = [tuple(p["x"], p["y"]) for p in make_polygon_mock.call_args.kwargs["point_path"]] expectation_points = [ (856.73076923, 1077.88461538), (575, 657.69230769), From c628046a77c5d9c8c711efc6db0432e7da19e187 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 16:32:28 +0000 Subject: [PATCH 13/21] Added type hints to help --- tests/darwin/importer/formats/import_dataloop_test.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 9103cf01c..b562add82 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -2,7 +2,7 @@ from math import isclose as math_isclose from os.path import dirname, join, realpath from pathlib import Path -from typing import Union +from typing import Dict, Tuple, Union from unittest import TestCase from unittest.mock import MagicMock, patch @@ -106,7 +106,10 @@ def test_handles_segment_type(self): with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - point_path = [tuple(p["x"], p["y"]) for p in make_polygon_mock.call_args.kwargs["point_path"]] + def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: + return (point["x"], point["y"]) + + point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] expectation_points = [ (856.73076923, 1077.88461538), (575, 657.69230769), From 410970c0e3cae2ad12f76f1393c1c030447a0357 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Mon, 28 Nov 2022 16:48:47 +0000 Subject: [PATCH 14/21] Fix for versions below 3.8 --- .../importer/formats/import_dataloop_test.py | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index b562add82..5e3febd80 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -106,24 +106,26 @@ def test_handles_segment_type(self): with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: - return (point["x"], point["y"]) - - point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] - expectation_points = [ - (856.73076923, 1077.88461538), - (575, 657.69230769), - (989.42307692, 409.61538462), - (974.03846154, 640.38461538), - (1033.65384615, 915.38461538), - (1106.73076923, 1053.84615385), - (1204.80769231, 1079.80769231), - ] - - [ - self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) - for a, b in zip(point_path, expectation_points) - ] + if "kwargs" in make_polygon_mock.call_args: + + def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: + return (point["x"], point["y"]) + + point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] + expectation_points = [ + (856.73076923, 1077.88461538), + (575, 657.69230769), + (989.42307692, 409.61538462), + (974.03846154, 640.38461538), + (1033.65384615, 915.38461538), + (1106.73076923, 1053.84615385), + (1204.80769231, 1079.80769231), + ] + + [ + self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) + for a, b in zip(point_path, expectation_points) + ] self.assertTrue(make_polygon_mock.call_args[0][0], "segment_class") def test_throws_on_unknown_type(self): From 227d0ec4295e6947cc85862e2cfca2793295d20f Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Wed, 30 Nov 2022 14:43:58 +0000 Subject: [PATCH 15/21] Added EOL --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 905c8bbe4..1fd489337 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,2 +1,2 @@ [flake8] -max-line-length = 160 \ No newline at end of file +max-line-length = 160 From e7538982c2ca7db182ffc88eb9c0ce316bc9dc77 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Thu, 1 Dec 2022 15:08:36 +0000 Subject: [PATCH 16/21] WIP Adding tests and fix. --- darwin/utils.py | 22 ++++--- tests/darwin/utils/find_files_test.py | 90 +++++++++++++++++++++++++++ tests/darwin/utils_test.py | 6 +- 3 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 tests/darwin/utils/find_files_test.py diff --git a/darwin/utils.py b/darwin/utils.py index 362e96cc1..c9965c5c9 100644 --- a/darwin/utils.py +++ b/darwin/utils.py @@ -49,21 +49,25 @@ SUPPORTED_EXTENSIONS = SUPPORTED_IMAGE_EXTENSIONS + SUPPORTED_VIDEO_EXTENSIONS -def is_extension_allowed(extension: str) -> bool: +def is_extension_allowed(filename: str) -> bool: """ Returns whether or not the given video or image extension is allowed. Parameters ---------- - extension : str - The extension. + filename : str + The filename. Returns ------- bool - Whether or not the given extension is allowed. + Whether or not the given extension of the filename is allowed. """ - return extension.lower() in SUPPORTED_EXTENSIONS + for ext in SUPPORTED_EXTENSIONS: + if filename.lower().endswith(ext): + return True + + return False def is_image_extension_allowed(extension: str) -> bool: @@ -210,13 +214,15 @@ def find_files( for f in files: path = Path(f) if path.is_dir(): - found_files.extend([f for f in path.glob(pattern) if is_extension_allowed("".join(f.suffixes))]) - elif is_extension_allowed("".join(path.suffixes)): + found_files.extend([f for f in path.glob(pattern) if is_extension_allowed(str(path))]) + elif is_extension_allowed(str(path)): found_files.append(path) else: raise UnsupportedFileType(path) - return [f for f in found_files if f not in map(Path, files_to_exclude)] + files_to_exclude_full_paths = [str(Path(f)) for f in files_to_exclude] + + return [f for f in found_files if str(f) not in files_to_exclude_full_paths] def secure_continue_request() -> bool: diff --git a/tests/darwin/utils/find_files_test.py b/tests/darwin/utils/find_files_test.py new file mode 100644 index 000000000..e79985fe8 --- /dev/null +++ b/tests/darwin/utils/find_files_test.py @@ -0,0 +1,90 @@ +from pathlib import Path, PosixPath +from typing import List +from unittest import TestCase, skip +from unittest.mock import MagicMock, patch + +from darwin.exceptions import UnsupportedFileType +from darwin.utils import SUPPORTED_EXTENSIONS, find_files, is_extension_allowed + + +class FindFileTestCase(TestCase): + fake_invalid_files = [ + "/testdir.invalidextension", + "/testdir/testdir2.invalidextension", + ] + fake_supported_files = [f"testdir/testfile{ext}" for ext in SUPPORTED_EXTENSIONS] + fake_supported_files_varied_case = [f"testdir/testdir2/testfile{ext.upper()}" for ext in SUPPORTED_EXTENSIONS] + fake_files = [ + "testdir/testdir2/testfile.png", + "testdir/testdir2/testfile2.png", + "testdir/testfile.png", + *fake_supported_files, + *fake_supported_files_varied_case, + ] + fake_file_expected_length = len(fake_files) - len(fake_invalid_files) + + def setUp(self) -> None: + return super().setUp() + + def tearDown(self) -> None: + return super().tearDown() + + +class TestFindFiles(FindFileTestCase): + @patch("darwin.utils.is_extension_allowed", return_value=True) + def test_find_files_returns_a_list_of_files(self, mock_is_extension_allowed): + output = find_files(self.fake_files, files_to_exclude=[], recursive=False) + + self.assertIsInstance(output, list) + [self.assertIsInstance(file, Path) for file in output] + + @patch("darwin.utils.is_extension_allowed", return_value=True) + def test_find_files_excludes_files_in_excluded_list(self, mock_is_extension_allowed): + output = find_files( + self.fake_files, + files_to_exclude=[ + "testdir/testdir2/testfile.png", + "testdir/testdir2/testfile2.png", + ], + recursive=False, + ) + + self.assertEqual(len(self.fake_files) - 2, len(output)) + + @patch("darwin.utils.is_extension_allowed", return_value=False) + def test_raises_error_unsupported_filetype(self, mock_is_extension_allowed): + with self.assertRaises(UnsupportedFileType): + find_files(["1"], files_to_exclude=[], recursive=False) + + @patch("darwin.utils.Path", autospec=True) + @patch("darwin.utils.is_extension_allowed") + def test_uses_correct_glob_if_recursive(self, mock_is_extension_allowed, mock_path): + mock_path.is_dir.return_value = True + mock_path.glob.return_value = ["1"] + + find_files(["1"], files_to_exclude=[], recursive=True) + + mock_path.return_value.glob.assert_called_once_with("**/*") + + @patch("darwin.utils.Path", autospec=True) + @patch("darwin.utils.is_extension_allowed") + def test_uses_correct_glob_if_not_recursive(self, mock_is_extension_allowed, mock_path): + mock_path.is_dir.return_value = True + mock_path.glob.return_value = ["1"] + + find_files(["1"], files_to_exclude=[], recursive=False) + + mock_path.return_value.glob.assert_called_once_with("*") + + +class TestIsExtensionAllowed(FindFileTestCase): + def test_returns_true_for_a_valid_extension(self): + for file in [*self.fake_supported_files, *self.fake_supported_files_varied_case]: + with self.subTest(file=file): + print(file) + self.assertTrue(is_extension_allowed(file)) + + def test_returns_false_for_an_invalid_extension(self): + for file in self.fake_invalid_files: + with self.subTest(file=file): + self.assertFalse(is_extension_allowed(file)) diff --git a/tests/darwin/utils_test.py b/tests/darwin/utils_test.py index 507078b92..e465f3fef 100644 --- a/tests/darwin/utils_test.py +++ b/tests/darwin/utils_test.py @@ -1,4 +1,5 @@ from unittest.mock import MagicMock, patch + from requests import Response import darwin.datatypes as dt @@ -470,11 +471,10 @@ def it_returns_json(): response: Response = Response() response.headers["content-type"] = "application/json" response._content = b'{"key":"a"}' - assert {'key':'a'} == get_response_content(response) + assert {"key": "a"} == get_response_content(response) def it_returns_text(): response: Response = Response() response.headers["content-type"] = "text/plain" - response._content = b'hello' + response._content = b"hello" assert "hello" == get_response_content(response) - From 5a06f2e59ea94f0b33213a4f4d8c439cf6c6e2d1 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Thu, 1 Dec 2022 17:08:59 +0000 Subject: [PATCH 17/21] Extra tests and functions --- darwin/utils.py | 78 ++++++++++++++++++++++++-- tests/darwin/utils/find_files_test.py | 80 +++++++++++++++++++++------ 2 files changed, 135 insertions(+), 23 deletions(-) diff --git a/darwin/utils.py b/darwin/utils.py index c9965c5c9..0ee0f990d 100644 --- a/darwin/utils.py +++ b/darwin/utils.py @@ -49,7 +49,7 @@ SUPPORTED_EXTENSIONS = SUPPORTED_IMAGE_EXTENSIONS + SUPPORTED_VIDEO_EXTENSIONS -def is_extension_allowed(filename: str) -> bool: +def is_extension_allowed_by_filename(filename: str) -> bool: """ Returns whether or not the given video or image extension is allowed. @@ -63,11 +63,43 @@ def is_extension_allowed(filename: str) -> bool: bool Whether or not the given extension of the filename is allowed. """ - for ext in SUPPORTED_EXTENSIONS: - if filename.lower().endswith(ext): - return True + return any([filename.lower().endswith(ext) for ext in SUPPORTED_EXTENSIONS]) - return False + +def is_extension_allowed(extension: str) -> bool: + """ + Returns whether or not the given extension is allowed. + @Deprecated. Use is_extension_allowed_by_filename instead, and pass full filename. + This is due to the fact that some extensions now include multiple dots, e.g. .nii.gz + + Parameters + ---------- + extension : str + The extension. + + Returns + ------- + bool + Whether or not the given extension is allowed. + """ + return extension.lower() in SUPPORTED_EXTENSIONS + + +def is_image_extension_allowed_by_filename(filename: str) -> bool: + """ + Returns whether or not the given image extension is allowed. + + Parameters + ---------- + filename : str + The image extension. + + Returns + ------- + bool + Whether or not the given extension is allowed. + """ + return any([filename.lower().endswith(ext) for ext in SUPPORTED_IMAGE_EXTENSIONS]) def is_image_extension_allowed(extension: str) -> bool: @@ -87,6 +119,23 @@ def is_image_extension_allowed(extension: str) -> bool: return extension.lower() in SUPPORTED_IMAGE_EXTENSIONS +def is_image_extension_allowed_by_filename(extension: str) -> bool: + """ + Returns whether or not the given image extension is allowed. + + Parameters + ---------- + extension : str + The image extension. + + Returns + ------- + bool + Whether or not the given extension is allowed. + """ + return any([extension.lower().endswith(ext) for ext in SUPPORTED_IMAGE_EXTENSIONS]) + + def is_video_extension_allowed(extension: str) -> bool: """ Returns whether or not the given video extension is allowed. @@ -104,6 +153,23 @@ def is_video_extension_allowed(extension: str) -> bool: return extension.lower() in SUPPORTED_VIDEO_EXTENSIONS +def is_video_extension_allowed_by_filename(filename: str) -> bool: + """ + Returns whether or not the given video extension is allowed. + + Parameters + ---------- + filename : str + The filename. + + Returns + ------- + bool + Whether or not the given extension is allowed. + """ + return any([filename.lower().endswith(ext) for ext in SUPPORTED_VIDEO_EXTENSIONS]) + + def urljoin(*parts: str) -> str: """ Take as input an unpacked list of strings and joins them to form an URL. @@ -215,7 +281,7 @@ def find_files( path = Path(f) if path.is_dir(): found_files.extend([f for f in path.glob(pattern) if is_extension_allowed(str(path))]) - elif is_extension_allowed(str(path)): + elif is_extension_allowed_by_filename(str(path)): found_files.append(path) else: raise UnsupportedFileType(path) diff --git a/tests/darwin/utils/find_files_test.py b/tests/darwin/utils/find_files_test.py index e79985fe8..68b46e631 100644 --- a/tests/darwin/utils/find_files_test.py +++ b/tests/darwin/utils/find_files_test.py @@ -1,10 +1,17 @@ +from dataclasses import dataclass from pathlib import Path, PosixPath -from typing import List +from typing import Any, Callable, Dict, List, Optional from unittest import TestCase, skip from unittest.mock import MagicMock, patch from darwin.exceptions import UnsupportedFileType -from darwin.utils import SUPPORTED_EXTENSIONS, find_files, is_extension_allowed +from darwin.utils import ( + SUPPORTED_EXTENSIONS, + SUPPORTED_IMAGE_EXTENSIONS, + SUPPORTED_VIDEO_EXTENSIONS, + find_files, + is_extension_allowed, +) class FindFileTestCase(TestCase): @@ -31,14 +38,14 @@ def tearDown(self) -> None: class TestFindFiles(FindFileTestCase): - @patch("darwin.utils.is_extension_allowed", return_value=True) + @patch("darwin.utils.is_extension_allowed_by_filename", return_value=True) def test_find_files_returns_a_list_of_files(self, mock_is_extension_allowed): output = find_files(self.fake_files, files_to_exclude=[], recursive=False) self.assertIsInstance(output, list) [self.assertIsInstance(file, Path) for file in output] - @patch("darwin.utils.is_extension_allowed", return_value=True) + @patch("darwin.utils.is_extension_allowed_by_filename", return_value=True) def test_find_files_excludes_files_in_excluded_list(self, mock_is_extension_allowed): output = find_files( self.fake_files, @@ -51,13 +58,13 @@ def test_find_files_excludes_files_in_excluded_list(self, mock_is_extension_allo self.assertEqual(len(self.fake_files) - 2, len(output)) - @patch("darwin.utils.is_extension_allowed", return_value=False) + @patch("darwin.utils.is_extension_allowed_by_filename", return_value=False) def test_raises_error_unsupported_filetype(self, mock_is_extension_allowed): with self.assertRaises(UnsupportedFileType): find_files(["1"], files_to_exclude=[], recursive=False) @patch("darwin.utils.Path", autospec=True) - @patch("darwin.utils.is_extension_allowed") + @patch("darwin.utils.is_extension_allowed_by_filename") def test_uses_correct_glob_if_recursive(self, mock_is_extension_allowed, mock_path): mock_path.is_dir.return_value = True mock_path.glob.return_value = ["1"] @@ -67,7 +74,7 @@ def test_uses_correct_glob_if_recursive(self, mock_is_extension_allowed, mock_pa mock_path.return_value.glob.assert_called_once_with("**/*") @patch("darwin.utils.Path", autospec=True) - @patch("darwin.utils.is_extension_allowed") + @patch("darwin.utils.is_extension_allowed_by_filename") def test_uses_correct_glob_if_not_recursive(self, mock_is_extension_allowed, mock_path): mock_path.is_dir.return_value = True mock_path.glob.return_value = ["1"] @@ -77,14 +84,53 @@ def test_uses_correct_glob_if_not_recursive(self, mock_is_extension_allowed, moc mock_path.return_value.glob.assert_called_once_with("*") -class TestIsExtensionAllowed(FindFileTestCase): - def test_returns_true_for_a_valid_extension(self): - for file in [*self.fake_supported_files, *self.fake_supported_files_varied_case]: - with self.subTest(file=file): - print(file) - self.assertTrue(is_extension_allowed(file)) +class TestIsExtensionAllowedByFilenameFunctions(FindFileTestCase): + @dataclass + class Dependencies: + ieabf: Optional[Callable[[str], bool]] = None + iveabf: Optional[Callable[[str], bool]] = None + iieabf: Optional[Callable[[str], bool]] = None - def test_returns_false_for_an_invalid_extension(self): - for file in self.fake_invalid_files: - with self.subTest(file=file): - self.assertFalse(is_extension_allowed(file)) + def dependency_factory(self) -> Dependencies: + """ + Dependency injection factory that allows different instantions of the dependencies + to avoid race condition failures and flaky tests. + + returns: Dependencies + """ + from darwin.utils import is_extension_allowed_by_filename as ieabf + from darwin.utils import is_image_extension_allowed_by_filename as iieabf + from darwin.utils import is_video_extension_allowed_by_filename as iveabf + + return self.Dependencies(ieabf=ieabf, iveabf=iveabf, iieabf=iieabf) + + def test_ieabf_returns_true_for_a_valid_extension(self): + valid_extensions = [*self.fake_supported_files, *self.fake_supported_files_varied_case] + results = [self.dependency_factory().ieabf(file) for file in valid_extensions] + + self.assertTrue(all(results)) + + def test_ieabf_returns_false_for_an_invalid_extension(self): + results = [self.dependency_factory().ieabf(file) for file in self.fake_invalid_files] + + self.assertFalse(all(results)) + + def test_iveabf_returns_true_for_a_valid_extension(self): + results = [self.dependency_factory().iveabf(file) for file in SUPPORTED_VIDEO_EXTENSIONS] + + self.assertTrue(all(results)) + + def test_iveabf_returns_false_for_an_invalid_extension(self): + results = [self.dependency_factory().iveabf(file) for file in self.fake_invalid_files] + + self.assertFalse(all(results)) + + def test_iieabf_returns_true_for_a_valid_extension(self): + results = [self.dependency_factory().iieabf(file) for file in SUPPORTED_IMAGE_EXTENSIONS] + + self.assertTrue(all(results)) + + def test_iieabf_returns_false_for_an_invalid_extension(self): + results = [self.dependency_factory().iieabf(file) for file in self.fake_invalid_files] + + self.assertFalse(all(results)) From 13518e108e0986de5524398ae29d6ef682ab79d6 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Fri, 2 Dec 2022 10:55:44 +0000 Subject: [PATCH 18/21] Revert "Extra tests and functions" This reverts commit 5a06f2e59ea94f0b33213a4f4d8c439cf6c6e2d1. --- darwin/utils.py | 78 ++------------------------ tests/darwin/utils/find_files_test.py | 80 ++++++--------------------- 2 files changed, 23 insertions(+), 135 deletions(-) diff --git a/darwin/utils.py b/darwin/utils.py index 0ee0f990d..c9965c5c9 100644 --- a/darwin/utils.py +++ b/darwin/utils.py @@ -49,7 +49,7 @@ SUPPORTED_EXTENSIONS = SUPPORTED_IMAGE_EXTENSIONS + SUPPORTED_VIDEO_EXTENSIONS -def is_extension_allowed_by_filename(filename: str) -> bool: +def is_extension_allowed(filename: str) -> bool: """ Returns whether or not the given video or image extension is allowed. @@ -63,43 +63,11 @@ def is_extension_allowed_by_filename(filename: str) -> bool: bool Whether or not the given extension of the filename is allowed. """ - return any([filename.lower().endswith(ext) for ext in SUPPORTED_EXTENSIONS]) + for ext in SUPPORTED_EXTENSIONS: + if filename.lower().endswith(ext): + return True - -def is_extension_allowed(extension: str) -> bool: - """ - Returns whether or not the given extension is allowed. - @Deprecated. Use is_extension_allowed_by_filename instead, and pass full filename. - This is due to the fact that some extensions now include multiple dots, e.g. .nii.gz - - Parameters - ---------- - extension : str - The extension. - - Returns - ------- - bool - Whether or not the given extension is allowed. - """ - return extension.lower() in SUPPORTED_EXTENSIONS - - -def is_image_extension_allowed_by_filename(filename: str) -> bool: - """ - Returns whether or not the given image extension is allowed. - - Parameters - ---------- - filename : str - The image extension. - - Returns - ------- - bool - Whether or not the given extension is allowed. - """ - return any([filename.lower().endswith(ext) for ext in SUPPORTED_IMAGE_EXTENSIONS]) + return False def is_image_extension_allowed(extension: str) -> bool: @@ -119,23 +87,6 @@ def is_image_extension_allowed(extension: str) -> bool: return extension.lower() in SUPPORTED_IMAGE_EXTENSIONS -def is_image_extension_allowed_by_filename(extension: str) -> bool: - """ - Returns whether or not the given image extension is allowed. - - Parameters - ---------- - extension : str - The image extension. - - Returns - ------- - bool - Whether or not the given extension is allowed. - """ - return any([extension.lower().endswith(ext) for ext in SUPPORTED_IMAGE_EXTENSIONS]) - - def is_video_extension_allowed(extension: str) -> bool: """ Returns whether or not the given video extension is allowed. @@ -153,23 +104,6 @@ def is_video_extension_allowed(extension: str) -> bool: return extension.lower() in SUPPORTED_VIDEO_EXTENSIONS -def is_video_extension_allowed_by_filename(filename: str) -> bool: - """ - Returns whether or not the given video extension is allowed. - - Parameters - ---------- - filename : str - The filename. - - Returns - ------- - bool - Whether or not the given extension is allowed. - """ - return any([filename.lower().endswith(ext) for ext in SUPPORTED_VIDEO_EXTENSIONS]) - - def urljoin(*parts: str) -> str: """ Take as input an unpacked list of strings and joins them to form an URL. @@ -281,7 +215,7 @@ def find_files( path = Path(f) if path.is_dir(): found_files.extend([f for f in path.glob(pattern) if is_extension_allowed(str(path))]) - elif is_extension_allowed_by_filename(str(path)): + elif is_extension_allowed(str(path)): found_files.append(path) else: raise UnsupportedFileType(path) diff --git a/tests/darwin/utils/find_files_test.py b/tests/darwin/utils/find_files_test.py index 68b46e631..e79985fe8 100644 --- a/tests/darwin/utils/find_files_test.py +++ b/tests/darwin/utils/find_files_test.py @@ -1,17 +1,10 @@ -from dataclasses import dataclass from pathlib import Path, PosixPath -from typing import Any, Callable, Dict, List, Optional +from typing import List from unittest import TestCase, skip from unittest.mock import MagicMock, patch from darwin.exceptions import UnsupportedFileType -from darwin.utils import ( - SUPPORTED_EXTENSIONS, - SUPPORTED_IMAGE_EXTENSIONS, - SUPPORTED_VIDEO_EXTENSIONS, - find_files, - is_extension_allowed, -) +from darwin.utils import SUPPORTED_EXTENSIONS, find_files, is_extension_allowed class FindFileTestCase(TestCase): @@ -38,14 +31,14 @@ def tearDown(self) -> None: class TestFindFiles(FindFileTestCase): - @patch("darwin.utils.is_extension_allowed_by_filename", return_value=True) + @patch("darwin.utils.is_extension_allowed", return_value=True) def test_find_files_returns_a_list_of_files(self, mock_is_extension_allowed): output = find_files(self.fake_files, files_to_exclude=[], recursive=False) self.assertIsInstance(output, list) [self.assertIsInstance(file, Path) for file in output] - @patch("darwin.utils.is_extension_allowed_by_filename", return_value=True) + @patch("darwin.utils.is_extension_allowed", return_value=True) def test_find_files_excludes_files_in_excluded_list(self, mock_is_extension_allowed): output = find_files( self.fake_files, @@ -58,13 +51,13 @@ def test_find_files_excludes_files_in_excluded_list(self, mock_is_extension_allo self.assertEqual(len(self.fake_files) - 2, len(output)) - @patch("darwin.utils.is_extension_allowed_by_filename", return_value=False) + @patch("darwin.utils.is_extension_allowed", return_value=False) def test_raises_error_unsupported_filetype(self, mock_is_extension_allowed): with self.assertRaises(UnsupportedFileType): find_files(["1"], files_to_exclude=[], recursive=False) @patch("darwin.utils.Path", autospec=True) - @patch("darwin.utils.is_extension_allowed_by_filename") + @patch("darwin.utils.is_extension_allowed") def test_uses_correct_glob_if_recursive(self, mock_is_extension_allowed, mock_path): mock_path.is_dir.return_value = True mock_path.glob.return_value = ["1"] @@ -74,7 +67,7 @@ def test_uses_correct_glob_if_recursive(self, mock_is_extension_allowed, mock_pa mock_path.return_value.glob.assert_called_once_with("**/*") @patch("darwin.utils.Path", autospec=True) - @patch("darwin.utils.is_extension_allowed_by_filename") + @patch("darwin.utils.is_extension_allowed") def test_uses_correct_glob_if_not_recursive(self, mock_is_extension_allowed, mock_path): mock_path.is_dir.return_value = True mock_path.glob.return_value = ["1"] @@ -84,53 +77,14 @@ def test_uses_correct_glob_if_not_recursive(self, mock_is_extension_allowed, moc mock_path.return_value.glob.assert_called_once_with("*") -class TestIsExtensionAllowedByFilenameFunctions(FindFileTestCase): - @dataclass - class Dependencies: - ieabf: Optional[Callable[[str], bool]] = None - iveabf: Optional[Callable[[str], bool]] = None - iieabf: Optional[Callable[[str], bool]] = None +class TestIsExtensionAllowed(FindFileTestCase): + def test_returns_true_for_a_valid_extension(self): + for file in [*self.fake_supported_files, *self.fake_supported_files_varied_case]: + with self.subTest(file=file): + print(file) + self.assertTrue(is_extension_allowed(file)) - def dependency_factory(self) -> Dependencies: - """ - Dependency injection factory that allows different instantions of the dependencies - to avoid race condition failures and flaky tests. - - returns: Dependencies - """ - from darwin.utils import is_extension_allowed_by_filename as ieabf - from darwin.utils import is_image_extension_allowed_by_filename as iieabf - from darwin.utils import is_video_extension_allowed_by_filename as iveabf - - return self.Dependencies(ieabf=ieabf, iveabf=iveabf, iieabf=iieabf) - - def test_ieabf_returns_true_for_a_valid_extension(self): - valid_extensions = [*self.fake_supported_files, *self.fake_supported_files_varied_case] - results = [self.dependency_factory().ieabf(file) for file in valid_extensions] - - self.assertTrue(all(results)) - - def test_ieabf_returns_false_for_an_invalid_extension(self): - results = [self.dependency_factory().ieabf(file) for file in self.fake_invalid_files] - - self.assertFalse(all(results)) - - def test_iveabf_returns_true_for_a_valid_extension(self): - results = [self.dependency_factory().iveabf(file) for file in SUPPORTED_VIDEO_EXTENSIONS] - - self.assertTrue(all(results)) - - def test_iveabf_returns_false_for_an_invalid_extension(self): - results = [self.dependency_factory().iveabf(file) for file in self.fake_invalid_files] - - self.assertFalse(all(results)) - - def test_iieabf_returns_true_for_a_valid_extension(self): - results = [self.dependency_factory().iieabf(file) for file in SUPPORTED_IMAGE_EXTENSIONS] - - self.assertTrue(all(results)) - - def test_iieabf_returns_false_for_an_invalid_extension(self): - results = [self.dependency_factory().iieabf(file) for file in self.fake_invalid_files] - - self.assertFalse(all(results)) + def test_returns_false_for_an_invalid_extension(self): + for file in self.fake_invalid_files: + with self.subTest(file=file): + self.assertFalse(is_extension_allowed(file)) From 3eaf12a182105b9771ca7016b02a415e255261b7 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Fri, 2 Dec 2022 10:56:04 +0000 Subject: [PATCH 19/21] Revert "WIP Adding tests and fix." This reverts commit e7538982c2ca7db182ffc88eb9c0ce316bc9dc77. --- darwin/utils.py | 22 +++---- tests/darwin/utils/find_files_test.py | 90 --------------------------- tests/darwin/utils_test.py | 6 +- 3 files changed, 11 insertions(+), 107 deletions(-) delete mode 100644 tests/darwin/utils/find_files_test.py diff --git a/darwin/utils.py b/darwin/utils.py index c9965c5c9..362e96cc1 100644 --- a/darwin/utils.py +++ b/darwin/utils.py @@ -49,25 +49,21 @@ SUPPORTED_EXTENSIONS = SUPPORTED_IMAGE_EXTENSIONS + SUPPORTED_VIDEO_EXTENSIONS -def is_extension_allowed(filename: str) -> bool: +def is_extension_allowed(extension: str) -> bool: """ Returns whether or not the given video or image extension is allowed. Parameters ---------- - filename : str - The filename. + extension : str + The extension. Returns ------- bool - Whether or not the given extension of the filename is allowed. + Whether or not the given extension is allowed. """ - for ext in SUPPORTED_EXTENSIONS: - if filename.lower().endswith(ext): - return True - - return False + return extension.lower() in SUPPORTED_EXTENSIONS def is_image_extension_allowed(extension: str) -> bool: @@ -214,15 +210,13 @@ def find_files( for f in files: path = Path(f) if path.is_dir(): - found_files.extend([f for f in path.glob(pattern) if is_extension_allowed(str(path))]) - elif is_extension_allowed(str(path)): + found_files.extend([f for f in path.glob(pattern) if is_extension_allowed("".join(f.suffixes))]) + elif is_extension_allowed("".join(path.suffixes)): found_files.append(path) else: raise UnsupportedFileType(path) - files_to_exclude_full_paths = [str(Path(f)) for f in files_to_exclude] - - return [f for f in found_files if str(f) not in files_to_exclude_full_paths] + return [f for f in found_files if f not in map(Path, files_to_exclude)] def secure_continue_request() -> bool: diff --git a/tests/darwin/utils/find_files_test.py b/tests/darwin/utils/find_files_test.py deleted file mode 100644 index e79985fe8..000000000 --- a/tests/darwin/utils/find_files_test.py +++ /dev/null @@ -1,90 +0,0 @@ -from pathlib import Path, PosixPath -from typing import List -from unittest import TestCase, skip -from unittest.mock import MagicMock, patch - -from darwin.exceptions import UnsupportedFileType -from darwin.utils import SUPPORTED_EXTENSIONS, find_files, is_extension_allowed - - -class FindFileTestCase(TestCase): - fake_invalid_files = [ - "/testdir.invalidextension", - "/testdir/testdir2.invalidextension", - ] - fake_supported_files = [f"testdir/testfile{ext}" for ext in SUPPORTED_EXTENSIONS] - fake_supported_files_varied_case = [f"testdir/testdir2/testfile{ext.upper()}" for ext in SUPPORTED_EXTENSIONS] - fake_files = [ - "testdir/testdir2/testfile.png", - "testdir/testdir2/testfile2.png", - "testdir/testfile.png", - *fake_supported_files, - *fake_supported_files_varied_case, - ] - fake_file_expected_length = len(fake_files) - len(fake_invalid_files) - - def setUp(self) -> None: - return super().setUp() - - def tearDown(self) -> None: - return super().tearDown() - - -class TestFindFiles(FindFileTestCase): - @patch("darwin.utils.is_extension_allowed", return_value=True) - def test_find_files_returns_a_list_of_files(self, mock_is_extension_allowed): - output = find_files(self.fake_files, files_to_exclude=[], recursive=False) - - self.assertIsInstance(output, list) - [self.assertIsInstance(file, Path) for file in output] - - @patch("darwin.utils.is_extension_allowed", return_value=True) - def test_find_files_excludes_files_in_excluded_list(self, mock_is_extension_allowed): - output = find_files( - self.fake_files, - files_to_exclude=[ - "testdir/testdir2/testfile.png", - "testdir/testdir2/testfile2.png", - ], - recursive=False, - ) - - self.assertEqual(len(self.fake_files) - 2, len(output)) - - @patch("darwin.utils.is_extension_allowed", return_value=False) - def test_raises_error_unsupported_filetype(self, mock_is_extension_allowed): - with self.assertRaises(UnsupportedFileType): - find_files(["1"], files_to_exclude=[], recursive=False) - - @patch("darwin.utils.Path", autospec=True) - @patch("darwin.utils.is_extension_allowed") - def test_uses_correct_glob_if_recursive(self, mock_is_extension_allowed, mock_path): - mock_path.is_dir.return_value = True - mock_path.glob.return_value = ["1"] - - find_files(["1"], files_to_exclude=[], recursive=True) - - mock_path.return_value.glob.assert_called_once_with("**/*") - - @patch("darwin.utils.Path", autospec=True) - @patch("darwin.utils.is_extension_allowed") - def test_uses_correct_glob_if_not_recursive(self, mock_is_extension_allowed, mock_path): - mock_path.is_dir.return_value = True - mock_path.glob.return_value = ["1"] - - find_files(["1"], files_to_exclude=[], recursive=False) - - mock_path.return_value.glob.assert_called_once_with("*") - - -class TestIsExtensionAllowed(FindFileTestCase): - def test_returns_true_for_a_valid_extension(self): - for file in [*self.fake_supported_files, *self.fake_supported_files_varied_case]: - with self.subTest(file=file): - print(file) - self.assertTrue(is_extension_allowed(file)) - - def test_returns_false_for_an_invalid_extension(self): - for file in self.fake_invalid_files: - with self.subTest(file=file): - self.assertFalse(is_extension_allowed(file)) diff --git a/tests/darwin/utils_test.py b/tests/darwin/utils_test.py index e465f3fef..507078b92 100644 --- a/tests/darwin/utils_test.py +++ b/tests/darwin/utils_test.py @@ -1,5 +1,4 @@ from unittest.mock import MagicMock, patch - from requests import Response import darwin.datatypes as dt @@ -471,10 +470,11 @@ def it_returns_json(): response: Response = Response() response.headers["content-type"] = "application/json" response._content = b'{"key":"a"}' - assert {"key": "a"} == get_response_content(response) + assert {'key':'a'} == get_response_content(response) def it_returns_text(): response: Response = Response() response.headers["content-type"] = "text/plain" - response._content = b"hello" + response._content = b'hello' assert "hello" == get_response_content(response) + From 882963e0a7c099e66945388e00c7f67e92e54227 Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Fri, 2 Dec 2022 13:57:13 +0000 Subject: [PATCH 20/21] Fix and linting --- darwin/exceptions.py | 25 ++- darwin/importer/formats/dataloop.py | 18 +- tests/darwin/data/dataloop.example.json | 165 +++++++++++++++--- .../importer/formats/import_dataloop_test.py | 47 ++--- 4 files changed, 200 insertions(+), 55 deletions(-) diff --git a/darwin/exceptions.py b/darwin/exceptions.py index a77aec1c1..2de4ca1c0 100644 --- a/darwin/exceptions.py +++ b/darwin/exceptions.py @@ -112,7 +112,7 @@ class Unauthorized(Exception): """ def __str__(self): - return f"Unauthorized" + return "Unauthorized" class OutdatedDarwinJSONFormat(Exception): @@ -158,6 +158,27 @@ def __init__(self, import_type: str, annotation_type: str): annotation_type: str The unsupported annotation type. """ - super().__init__(f"Unsupported annotation type {annotation_type} for {import_type} import") + super().__init__( + f"Unsupported annotation type {annotation_type} for {import_type} import" + ) self.import_type = import_type self.annotation_type = annotation_type + + +class DataloopComplexPolygonsNotYetSupported(Exception): + """ + Used when one tries to parse an annotation with a complex polygon. + """ + + def __init__( + self, + ): + """ + Parameters + ---------- + import_type: str + The type of import, e.g. "dataloop". + annotation_type: str + The unsupported annotation type. + """ + super().__init__("Complex polygons not yet supported for dataloop import") diff --git a/darwin/importer/formats/dataloop.py b/darwin/importer/formats/dataloop.py index c24243a07..0ca78eff7 100644 --- a/darwin/importer/formats/dataloop.py +++ b/darwin/importer/formats/dataloop.py @@ -3,7 +3,10 @@ from typing import Any, Dict, List, Optional, Set import darwin.datatypes as dt -from darwin.exceptions import UnsupportedImportAnnotationType +from darwin.exceptions import ( + DataloopComplexPolygonsNotYetSupported, + UnsupportedImportAnnotationType, +) def parse_path(path: Path) -> Optional[dt.AnnotationFile]: @@ -26,8 +29,12 @@ def parse_path(path: Path) -> Optional[dt.AnnotationFile]: return None with path.open() as f: data = json.load(f) - annotations: List[dt.Annotation] = list(filter(None, map(_parse_annotation, data["annotations"]))) - annotation_classes: Set[dt.AnnotationClass] = set([annotation.annotation_class for annotation in annotations]) + annotations: List[dt.Annotation] = list( + filter(None, map(_parse_annotation, data["annotations"])) + ) + annotation_classes: Set[dt.AnnotationClass] = set( + [annotation.annotation_class for annotation in annotations] + ) return dt.AnnotationFile( path, _remove_leading_slash(data["filename"]), @@ -65,7 +72,10 @@ def _parse_annotation(annotation: Dict[str, Any]) -> Optional[dt.Annotation]: if annotation_type == "segment": coords = annotation["coordinates"] - points: List[dt.Point] = [{"x": c["x"], "y": c["y"]} for c in coords] + if len(coords) != 1: + raise DataloopComplexPolygonsNotYetSupported() + + points: List[dt.Point] = [{"x": c["x"], "y": c["y"]} for c in coords[0]] return dt.make_polygon(annotation_label, point_path=points) return None diff --git a/tests/darwin/data/dataloop.example.json b/tests/darwin/data/dataloop.example.json index f95d80e6b..64ff82378 100644 --- a/tests/darwin/data/dataloop.example.json +++ b/tests/darwin/data/dataloop.example.json @@ -107,34 +107,36 @@ "label": "segment_class", "attributes": [], "coordinates": [ - { - "x": 856.7307692307692, - "y": 1077.8846153846152 - }, - { - "x": 575, - "y": 657.6923076923076 - }, - { - "x": 989.4230769230768, - "y": 409.6153846153846 - }, - { - "x": 974.0384615384614, - "y": 640.3846153846154 - }, - { - "x": 1033.653846153846, - "y": 915.3846153846154 - }, - { - "x": 1106.730769230769, - "y": 1053.8461538461538 - }, - { - "x": 1204.8076923076922, - "y": 1079.8076923076922 - } + [ + { + "x": 856.7307692307692, + "y": 1077.8846153846152 + }, + { + "x": 575, + "y": 657.6923076923076 + }, + { + "x": 989.4230769230768, + "y": 409.6153846153846 + }, + { + "x": 974.0384615384614, + "y": 640.3846153846154 + }, + { + "x": 1033.653846153846, + "y": 915.3846153846154 + }, + { + "x": 1106.730769230769, + "y": 1053.8461538461538 + }, + { + "x": 1204.8076923076922, + "y": 1079.8076923076922 + } + ] ], "metadata": { "system": { @@ -249,6 +251,113 @@ "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", "hash": "e8726832771219919976179815544bba59bf15d5", "source": "ui" + }, + { + "id": "5", + "datasetId": "61d5e83f5575066ffd58fcda", + "type": "segment", + "label": "segment_class", + "attributes": [], + "coordinates": [ + [ + { + "x": 856.7307692307692, + "y": 1077.8846153846152 + }, + { + "x": 575, + "y": 657.6923076923076 + }, + { + "x": 989.4230769230768, + "y": 409.6153846153846 + }, + { + "x": 974.0384615384614, + "y": 640.3846153846154 + }, + { + "x": 1033.653846153846, + "y": 915.3846153846154 + }, + { + "x": 1106.730769230769, + "y": 1053.8461538461538 + }, + { + "x": 1204.8076923076922, + "y": 1079.8076923076922 + } + ], + [ + { + "x": 856.7307692307692, + "y": 1077.8846153846152 + }, + { + "x": 575, + "y": 657.6923076923076 + }, + { + "x": 989.4230769230768, + "y": 409.6153846153846 + }, + { + "x": 974.0384615384614, + "y": 640.3846153846154 + }, + { + "x": 1033.653846153846, + "y": 915.3846153846154 + }, + { + "x": 1106.730769230769, + "y": 1053.8461538461538 + }, + { + "x": 1204.8076923076922, + "y": 1079.8076923076922 + } + ] + ], + "metadata": { + "system": { + "status": null, + "startTime": 0, + "endTime": 1, + "frame": 0, + "endFrame": 1, + "snapshots_": [], + "parentId": null, + "clientId": "ab0eaf4b-4dc5-471c-921f-f8928d54d1a1", + "automated": false, + "objectId": "2", + "isOpen": false, + "isOnlyLocal": false, + "attributes": { + "1": false + }, + "clientParentId": null, + "system": false, + "description": null, + "itemLinks": [], + "openAnnotationVersion": "1.29.1-rc.8", + "recipeId": "61d5e83f71a8721c42b8f02e", + "taskId": "61d5eb2f3e1b9e55742d498c", + "assignmentId": "61d5eb2f3e1b9eb7c22d498d" + }, + "user": {} + }, + "creator": "vakasix267@zoeyy.com", + "createdAt": "2022-01-05T19:26:23.252Z", + "updatedBy": "vakasix267@zoeyy.com", + "updatedAt": "2022-01-05T19:26:23.252Z", + "itemId": "61d5eb9d14b5b8ea83e18037", + "url": "https://rc-gate.dataloop.ai/api/v1/annotations/61d5f0dfebda8c3885b7eae5", + "item": "https://rc-gate.dataloop.ai/api/v1/items/61d5eb9d14b5b8ea83e18037", + "dataset": "https://rc-gate.dataloop.ai/api/v1/datasets/61d5e83f5575066ffd58fcda", + "hash": "e8726832771219919976179815544bba59bf15d5", + "source": "ui" } ] } \ No newline at end of file diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index 5e3febd80..d722a7049 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -7,7 +7,10 @@ from unittest.mock import MagicMock, patch from darwin.datatypes import Annotation -from darwin.exceptions import UnsupportedImportAnnotationType +from darwin.exceptions import ( + DataloopComplexPolygonsNotYetSupported, + UnsupportedImportAnnotationType, +) from darwin.importer.formats.dataloop import ( _parse_annotation, _remove_leading_slash, @@ -106,26 +109,24 @@ def test_handles_segment_type(self): with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - if "kwargs" in make_polygon_mock.call_args: - - def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: - return (point["x"], point["y"]) - - point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] - expectation_points = [ - (856.73076923, 1077.88461538), - (575, 657.69230769), - (989.42307692, 409.61538462), - (974.03846154, 640.38461538), - (1033.65384615, 915.38461538), - (1106.73076923, 1053.84615385), - (1204.80769231, 1079.80769231), - ] - - [ - self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) - for a, b in zip(point_path, expectation_points) - ] + def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: + return (point["x"], point["y"]) + + point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] + expectation_points = [ + (856.73076923, 1077.88461538), + (575, 657.69230769), + (989.42307692, 409.61538462), + (974.03846154, 640.38461538), + (1033.65384615, 915.38461538), + (1106.73076923, 1053.84615385), + (1204.80769231, 1079.80769231), + ] + + [ + self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) + for a, b in zip(point_path, expectation_points) + ] self.assertTrue(make_polygon_mock.call_args[0][0], "segment_class") def test_throws_on_unknown_type(self): @@ -136,3 +137,7 @@ def test_throws_on_unknown_type(self): self.assertEqual(e.annotation_type, "UNSUPPORTED_TYPE") except Exception as e: self.fail(f"Test threw wrong exception: {e}") + + def test_rejects_complex_polygons(self): + with self.assertRaises(DataloopComplexPolygonsNotYetSupported): + _parse_annotation(self.parsed_json["annotations"][4]) # 4 is a complex polygon From 99cc693727d8e3275c62a32b1ca2d0da2badb5ef Mon Sep 17 00:00:00 2001 From: Owen Jones Date: Fri, 2 Dec 2022 14:20:30 +0000 Subject: [PATCH 21/21] Test fix for Py 3.8 --- .../importer/formats/import_dataloop_test.py | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/tests/darwin/importer/formats/import_dataloop_test.py b/tests/darwin/importer/formats/import_dataloop_test.py index d722a7049..2ef4a2ad6 100644 --- a/tests/darwin/importer/formats/import_dataloop_test.py +++ b/tests/darwin/importer/formats/import_dataloop_test.py @@ -109,24 +109,26 @@ def test_handles_segment_type(self): with patch("darwin.importer.formats.dataloop.dt.make_polygon") as make_polygon_mock: pa(self.parsed_json["annotations"][2]) # 2 is a segment type - def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: - return (point["x"], point["y"]) - - point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] - expectation_points = [ - (856.73076923, 1077.88461538), - (575, 657.69230769), - (989.42307692, 409.61538462), - (974.03846154, 640.38461538), - (1033.65384615, 915.38461538), - (1106.73076923, 1053.84615385), - (1204.80769231, 1079.80769231), - ] - - [ - self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) - for a, b in zip(point_path, expectation_points) - ] + if "kwargs" in make_polygon_mock.call_args: + + def make_tuple_entry(point: Dict[str, float]) -> Tuple[float, float]: + return (point["x"], point["y"]) + + point_path = [make_tuple_entry(p) for p in make_polygon_mock.call_args.kwargs["point_path"]] + expectation_points = [ + (856.73076923, 1077.88461538), + (575, 657.69230769), + (989.42307692, 409.61538462), + (974.03846154, 640.38461538), + (1033.65384615, 915.38461538), + (1106.73076923, 1053.84615385), + (1204.80769231, 1079.80769231), + ] + + [ + self.assertApproximatelyEqualNumber(a[0], b[0]) and self.assertApproximatelyEqualNumber(a[1], b[1]) + for a, b in zip(point_path, expectation_points) + ] self.assertTrue(make_polygon_mock.call_args[0][0], "segment_class") def test_throws_on_unknown_type(self):