Different parsed types between `--element` and `--elements-list`

arkindex_worker/worker/__init__.py

@@ -90,7 +90,7 @@ class ElementsWorker(
         )
         self.parser.add_argument(
             "--element",
-            type=uuid.UUID,
+            type=str,
             nargs="+",
             help="One or more Arkindex element ID",
         )
@@ -109,11 +109,23 @@ class ElementsWorker(
         the cache database when enabled.
 
         :return: An iterable of [CachedElement][arkindex_worker.cache.CachedElement] when cache support is enabled,
-           and a list of strings representing element IDs otherwise.
+           or a list of strings representing element IDs otherwise.
         """
         assert not (
             self.args.elements_list and self.args.element
         ), "elements-list and element CLI args shouldn't be both set"
+
+        def invalid_element_id(value: str) -> bool:
+            """
+            Return whether the ID of an element is a valid UUID or not
+            """
+            try:
+                uuid.UUID(value)
+            except Exception:
+                return True
+
+            return False
+
         out = []
 
         # Load from the cache when available
@@ -133,6 +145,11 @@ class ElementsWorker(
         elif self.args.element:
             out += self.args.element
 
+        invalid_element_ids = list(filter(invalid_element_id, out))
+        assert (
+            not invalid_element_ids
+        ), f"These element IDs are invalid: {', '.join(invalid_element_ids)}"
+
         return out
 
     @property

tests/test_base_worker.py

@@ -669,8 +669,7 @@ def test_find_parents_file_paths(responses, mock_base_worker_with_cache, tmp_pat
     ):
         (tmp_path / parent_id).mkdir()
         file_path = tmp_path / parent_id / filename
-        with file_path.open("w", encoding="utf-8") as f:
-            f.write(content)
+        file_path.write_text(content)
 
     # Configure worker with a specific data directory
     mock_base_worker_with_cache.task_data_dir = tmp_path

tests/test_elements_worker/test_cli.py

@@ -2,7 +2,6 @@ import json
 import sys
 import tempfile
 from pathlib import Path
-from uuid import UUID
 
 import pytest
 
@@ -58,13 +57,6 @@ def test_cli_arg_elements_list_given(mocker):
     path.unlink()
 
 
-def test_cli_arg_element_one_given_not_uuid(mocker):
-    mocker.patch.object(sys, "argv", ["worker", "--element", "1234"])
-    worker = ElementsWorker()
-    with pytest.raises(SystemExit):
-        worker.configure()
-
-
 @pytest.mark.usefixtures("_mock_worker_run_api")
 def test_cli_arg_element_one_given(mocker):
     mocker.patch.object(
@@ -73,7 +65,7 @@ def test_cli_arg_element_one_given(mocker):
     worker = ElementsWorker()
     worker.configure()
 
-    assert worker.args.element == [UUID("12341234-1234-1234-1234-123412341234")]
+    assert worker.args.element == ["12341234-1234-1234-1234-123412341234"]
     # elements_list is None because TASK_ELEMENTS environment variable isn't set
     assert not worker.args.elements_list
 
@@ -94,8 +86,8 @@ def test_cli_arg_element_many_given(mocker):
     worker.configure()
 
     assert worker.args.element == [
-        UUID("12341234-1234-1234-1234-123412341234"),
-        UUID("43214321-4321-4321-4321-432143214321"),
+        "12341234-1234-1234-1234-123412341234",
+        "43214321-4321-4321-4321-432143214321",
     ]
     # elements_list is None because TASK_ELEMENTS environment variable isn't set
     assert not worker.args.elements_list

tests/test_elements_worker/test_elements.py

@@ -135,8 +135,7 @@ def test_list_elements_elements_list_arg_missing_id(
     monkeypatch, tmp_path, mock_elements_worker
 ):
     elements_path = tmp_path / "elements.json"
-    with elements_path.open("w") as f:
-        json.dump([{"type": "volume"}], f)
+    elements_path.write_text(json.dumps([{"type": "volume"}]))
 
     monkeypatch.setenv("TASK_ELEMENTS", str(elements_path))
     worker = ElementsWorker()
@@ -147,18 +146,43 @@ def test_list_elements_elements_list_arg_missing_id(
     assert elt_list == []
 
 
-def test_list_elements_elements_list_arg(monkeypatch, tmp_path, mock_elements_worker):
+def test_list_elements_elements_list_arg_not_uuid(
+    monkeypatch, tmp_path, mock_elements_worker
+):
     elements_path = tmp_path / "elements.json"
-    with elements_path.open("w") as f:
-        json.dump(
+    elements_path.write_text(
+        json.dumps(
             [
                 {"id": "volumeid", "type": "volume"},
                 {"id": "pageid", "type": "page"},
                 {"id": "actid", "type": "act"},
                 {"id": "surfaceid", "type": "surface"},
-            ],
-            f,
+            ]
+        )
+    )
+
+    monkeypatch.setenv("TASK_ELEMENTS", str(elements_path))
+    worker = ElementsWorker()
+    worker.configure()
+
+    with pytest.raises(
+        Exception,
+        match="These element IDs are invalid: volumeid, pageid, actid, surfaceid",
+    ):
+        worker.list_elements()
+
+
+def test_list_elements_elements_list_arg(monkeypatch, tmp_path, mock_elements_worker):
+    elements_path = tmp_path / "elements.json"
+    elements_path.write_text(
+        json.dumps(
+            [
+                {"id": "11111111-1111-1111-1111-111111111111", "type": "volume"},
+                {"id": "22222222-2222-2222-2222-222222222222", "type": "page"},
+                {"id": "33333333-3333-3333-3333-333333333333", "type": "act"},
+            ]
         )
+    )
 
     monkeypatch.setenv("TASK_ELEMENTS", str(elements_path))
     worker = ElementsWorker()
@@ -166,10 +190,14 @@ def test_list_elements_elements_list_arg(monkeypatch, tmp_path, mock_elements_wo
 
     elt_list = worker.list_elements()
 
-    assert elt_list == ["volumeid", "pageid", "actid", "surfaceid"]
+    assert elt_list == [
+        "11111111-1111-1111-1111-111111111111",
+        "22222222-2222-2222-2222-222222222222",
+        "33333333-3333-3333-3333-333333333333",
+    ]
 
 
-def test_list_elements_element_arg(mocker, mock_elements_worker):
+def test_list_elements_element_arg_not_uuid(mocker, mock_elements_worker):
     mocker.patch(
         "arkindex_worker.worker.base.argparse.ArgumentParser.parse_args",
         return_value=Namespace(
@@ -184,23 +212,50 @@ def test_list_elements_element_arg(mocker, mock_elements_worker):
     worker = ElementsWorker()
     worker.configure()
 
+    with pytest.raises(
+        Exception, match="These element IDs are invalid: volumeid, pageid"
+    ):
+        worker.list_elements()
+
+
+def test_list_elements_element_arg(mocker, mock_elements_worker):
+    mocker.patch(
+        "arkindex_worker.worker.base.argparse.ArgumentParser.parse_args",
+        return_value=Namespace(
+            element=[
+                "11111111-1111-1111-1111-111111111111",
+                "22222222-2222-2222-2222-222222222222",
+            ],
+            verbose=False,
+            elements_list=None,
+            database=None,
+            dev=False,
+        ),
+    )
+
+    worker = ElementsWorker()
+    worker.configure()
+
     elt_list = worker.list_elements()
 
-    assert elt_list == ["volumeid", "pageid"]
+    assert elt_list == [
+        "11111111-1111-1111-1111-111111111111",
+        "22222222-2222-2222-2222-222222222222",
+    ]
 
 
 def test_list_elements_both_args_error(mocker, mock_elements_worker, tmp_path):
     elements_path = tmp_path / "elements.json"
-    with elements_path.open("w") as f:
-        json.dump(
+    elements_path.write_text(
+        json.dumps(
             [
                 {"id": "volumeid", "type": "volume"},
                 {"id": "pageid", "type": "page"},
                 {"id": "actid", "type": "act"},
                 {"id": "surfaceid", "type": "surface"},
-            ],
-            f,
+            ]
         )
+    )
     mocker.patch(
         "arkindex_worker.worker.base.argparse.ArgumentParser.parse_args",
         return_value=Namespace(

tests/test_image.py

@@ -47,8 +47,7 @@ def _root_mean_square(img_a, img_b):
 
 def test_download_tiles(responses):
     expected = Image.open(FULL_IMAGE).convert("RGB")
-    with TILE.open("rb") as tile:
-        tile_bytes = tile.read()
+    tile_bytes = TILE.read_bytes()
 
     responses.add(
         responses.GET,
@@ -76,9 +75,8 @@ def test_download_tiles_crop(responses):
     """
     expected = Image.open(FULL_IMAGE).convert("RGB")
     tile_bytes = BytesIO()
-    with TILE.open("rb") as tile:
-        # Add one extra pixel to each tile to return slightly bigger tiles
-        ImageOps.pad(Image.open(tile), (181, 241)).save(tile_bytes, format="jpeg")
+    # Add one extra pixel to each tile to return slightly bigger tiles
+    ImageOps.pad(Image.open(TILE), (181, 241)).save(tile_bytes, format="jpeg")
 
     tile_bytes = tile_bytes.getvalue()