.gitlab-ci.yml

@@ -25,7 +25,7 @@ lint:
     - pre-commit run -a
 
 test:
-  image: python:3
+  image: python:3.11
   stage: test
 
   cache:
@@ -55,7 +55,7 @@ test:
     - tox -- --junitxml=test-report.xml --durations=50
 
 test-cookiecutter:
-  image: python:3
+  image: python:3.11
 
   stage: test
   cache:
@@ -136,7 +136,7 @@ pypi-publication:
     - twine upload dist/* -r pypi
 
 .docs:
-  image: python:3
+  image: python:3.11
   artifacts:
     paths:
       - public

arkindex_worker/worker/element.py

@@ -3,6 +3,7 @@
 ElementsWorker methods for elements and element types.
 """
 from typing import Dict, Iterable, List, NamedTuple, Optional, Union
+from uuid import UUID
 
 from peewee import IntegrityError
 
@@ -275,33 +276,39 @@ class ElementMixin(object):
 
         return created_ids
 
-    def update_element(
-        self,
-        element: Union[Element, CachedElement],
-        type: Optional[str] = None,
-        name: Optional[str] = None,
-        polygon: Optional[List[List[Union[int, float]]]] = None,
-        confidence: Optional[float] = None,
+    def partial_update_element(
+        self, element: Union[Element, CachedElement], **kwargs
     ) -> dict:
         """
-        Partially update an element through the API.
+        Partially updates an element through the API.
 
         :param element: The element to update.
-        :param type: Optional new slug type of the element.
-        :param name: Optional new name of the element.
-        :param polygon: Optional new polygon of the element.
-        :param confidence: Optional new confidence score, between 0.0 and 1.0.
+        :param **kwargs:
+
+            * *type* (``str``): Optional slug type of the element.
+            * *name* (``str``): Optional name of the element.
+            * *polygon* (``list``): Optional polygon for this element
+            * *confidence* (``float``): Optional confidence score of this element
+            * *rotation_angle* (``int``): Optional rotation angle of this element
+            * *mirrored* (``bool``): Optional mirror status of this element
+            * *image* (``UUID``): Optional ID of the image of this element
+
+
         :returns: A dict from the ``PartialUpdateElement`` API endpoint,
         """
         assert element and isinstance(
             element, (Element, CachedElement)
         ), "element shouldn't be null and should be an Element or CachedElement"
-        assert type is None or isinstance(type, str), "type should be None or a str"
-        assert name is None or isinstance(name, str), "name should be None or a str"
-        assert polygon is None or isinstance(
-            polygon, list
-        ), "polygon should be None or a list"
-        if polygon:
+
+        if "type" in kwargs:
+            assert isinstance(kwargs["type"], str), "type should be a str"
+
+        if "name" in kwargs:
+            assert isinstance(kwargs["name"], str), "name should be a str"
+
+        if "polygon" in kwargs:
+            polygon = kwargs["polygon"]
+            assert isinstance(polygon, list), "polygon should be a list"
             assert len(polygon) >= 3, "polygon should have at least three points"
             assert all(
                 isinstance(point, list) and len(point) == 2 for point in polygon
@@ -309,9 +316,27 @@ class ElementMixin(object):
             assert all(
                 isinstance(coord, (int, float)) for point in polygon for coord in point
             ), "polygon points should be lists of two numbers"
-        assert confidence is None or (
-            isinstance(confidence, float) and 0 <= confidence <= 1
-        ), "confidence should be None or a float in [0..1] range"
+
+        if "confidence" in kwargs:
+            confidence = kwargs["confidence"]
+            assert confidence is None or (
+                isinstance(confidence, float) and 0 <= confidence <= 1
+            ), "confidence should be None or a float in [0..1] range"
+
+        if "rotation_angle" in kwargs:
+            rotation_angle = kwargs["rotation_angle"]
+            assert (
+                isinstance(rotation_angle, int) and rotation_angle >= 0
+            ), "rotation_angle should be a positive integer"
+
+        if "mirrored" in kwargs:
+            assert isinstance(kwargs["mirrored"], bool), "mirrored should be a boolean"
+
+        if "image" in kwargs:
+            image = kwargs["image"]
+            assert isinstance(image, UUID), "image should be a UUID"
+            # Cast to string
+            kwargs["image"] = str(image)
 
         if self.is_read_only:
             logger.warning("Cannot update element as this worker is in read-only mode")
@@ -320,22 +345,24 @@ class ElementMixin(object):
         updated_element = self.request(
             "PartialUpdateElement",
             id=element.id,
-            body={
-                "type": type,
-                "name": name,
-                "polygon": polygon,
-                "confidence": confidence,
-            },
+            body=kwargs,
         )
 
         if self.use_cache:
-            CachedElement.update(
-                {
-                    CachedElement.type: type,
-                    CachedElement.polygon: str(polygon),
-                    CachedElement.confidence: confidence,
-                }
-            ).where(CachedElement.id == element.id).execute()
+            # Name is not present in CachedElement model
+            kwargs.pop("name", None)
+
+            # Stringify polygon if present
+            if "polygon" in kwargs:
+                kwargs["polygon"] = str(kwargs["polygon"])
+
+            # Retrieve the right image
+            if "image" in kwargs:
+                kwargs["image"] = CachedImage.get_by_id(kwargs["image"])
+
+            CachedElement.update(**kwargs).where(
+                CachedElement.id == element.id
+            ).execute()
 
         return updated_element
 

requirements.txt

-arkindex-client==1.0.13
+arkindex-client==1.0.14
 peewee==3.16.3
 Pillow==10.0.0
 pymdown-extensions==10.2

tests/conftest.py

@@ -16,6 +16,7 @@ from arkindex_worker.cache import (
     MODELS,
     SQL_VERSION,
     CachedElement,
+    CachedImage,
     CachedTranscription,
     Version,
     create_version_table,
@@ -363,6 +364,18 @@ def mock_cached_elements():
     assert CachedElement.select().count() == 5
 
 
+@pytest.fixture
+def mock_cached_images():
+    """Insert few elements in local cache"""
+    CachedImage.create(
+        id=UUID("99999999-9999-9999-9999-999999999999"),
+        width=1250,
+        height=2500,
+        url="http://testserver/iiif/3/image",
+    )
+    assert CachedImage.select().count() == 1
+
+
 @pytest.fixture
 def mock_cached_transcriptions():
     """Insert few transcriptions in local cache, on a shared element"""

tests/test_elements_worker/test_elements.py

@@ -1210,94 +1210,208 @@ def test_create_elements_integrity_error(
     assert list(CachedElement.select()) == []
 
 
-def test_update_element_wrong_element(mock_elements_worker):
-    with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=None,
-        )
-    assert (
-        str(e.value)
-        == "element shouldn't be null and should be an Element or CachedElement"
-    )
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Element
+        (
+            {"element": None},
+            "element shouldn't be null and should be an Element or CachedElement",
+        ),
+        (
+            {"element": "not element type"},
+            "element shouldn't be null and should be an Element or CachedElement",
+        ),
+    ),
+)
+def test_partial_update_element_wrong_param_element(
+    mock_elements_worker, payload, error
+):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
 
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element="not element type",
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert (
-        str(e.value)
-        == "element shouldn't be null and should be an Element or CachedElement"
-    )
+    assert str(e.value) == error
 
 
-def test_update_element_wrong_type(mock_elements_worker):
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Type
+        ({"type": 1234}, "type should be a str"),
+        ({"type": None}, "type should be a str"),
+    ),
+)
+def test_partial_update_element_wrong_param_type(mock_elements_worker, payload, error):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
+
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=Element({"zone": None}),
-            type=1234,
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "type should be None or a str"
+    assert str(e.value) == error
 
 
-def test_update_element_wrong_name(mock_elements_worker):
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Name
+        ({"name": 1234}, "name should be a str"),
+        ({"name": None}, "name should be a str"),
+    ),
+)
+def test_partial_update_element_wrong_param_name(mock_elements_worker, payload, error):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
+
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=Element({"zone": None}),
-            name=1234,
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "name should be None or a str"
+    assert str(e.value) == error
 
 
-def test_update_element_wrong_polygon(mock_elements_worker):
-    elt = Element({"zone": None})
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Polygon
+        ({"polygon": "not a polygon"}, "polygon should be a list"),
+        ({"polygon": None}, "polygon should be a list"),
+        ({"polygon": [[1, 1], [2, 2]]}, "polygon should have at least three points"),
+        (
+            {"polygon": [[1, 1, 1], [2, 2, 1], [2, 1, 1], [1, 2, 1]]},
+            "polygon points should be lists of two items",
+        ),
+        (
+            {"polygon": [[1], [2], [2], [1]]},
+            "polygon points should be lists of two items",
+        ),
+        (
+            {"polygon": [["not a coord", 1], [2, 2], [2, 1], [1, 2]]},
+            "polygon points should be lists of two numbers",
+        ),
+    ),
+)
+def test_partial_update_element_wrong_param_polygon(
+    mock_elements_worker, payload, error
+):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
 
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=elt,
-            polygon="not a polygon",
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "polygon should be None or a list"
+    assert str(e.value) == error
 
-    with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=elt,
-            polygon=[[1, 1], [2, 2]],
-        )
-    assert str(e.value) == "polygon should have at least three points"
+
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Confidence
+        ({"confidence": "lol"}, "confidence should be None or a float in [0..1] range"),
+        ({"confidence": "0.2"}, "confidence should be None or a float in [0..1] range"),
+        ({"confidence": -1.0}, "confidence should be None or a float in [0..1] range"),
+        ({"confidence": 1.42}, "confidence should be None or a float in [0..1] range"),
+        (
+            {"confidence": float("inf")},
+            "confidence should be None or a float in [0..1] range",
+        ),
+    ),
+)
+def test_partial_update_element_wrong_param_conf(mock_elements_worker, payload, error):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
 
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=elt,
-            polygon=[[1, 1, 1], [2, 2, 1], [2, 1, 1], [1, 2, 1]],
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "polygon points should be lists of two items"
+    assert str(e.value) == error
+
+
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Rotation angle
+        ({"rotation_angle": "lol"}, "rotation_angle should be a positive integer"),
+        ({"rotation_angle": -1}, "rotation_angle should be a positive integer"),
+        ({"rotation_angle": 0.5}, "rotation_angle should be a positive integer"),
+        ({"rotation_angle": None}, "rotation_angle should be a positive integer"),
+    ),
+)
+def test_partial_update_element_wrong_param_rota(mock_elements_worker, payload, error):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
 
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=elt,
-            polygon=[[1], [2], [2], [1]],
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "polygon points should be lists of two items"
+    assert str(e.value) == error
+
+
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Mirrored
+        ({"mirrored": "lol"}, "mirrored should be a boolean"),
+        ({"mirrored": 1234}, "mirrored should be a boolean"),
+        ({"mirrored": None}, "mirrored should be a boolean"),
+    ),
+)
+def test_partial_update_element_wrong_param_mir(mock_elements_worker, payload, error):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
 
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=elt,
-            polygon=[["not a coord", 1], [2, 2], [2, 1], [1, 2]],
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "polygon points should be lists of two numbers"
+    assert str(e.value) == error
 
 
-@pytest.mark.parametrize("confidence", ["lol", "0.2", -1.0, 1.42, float("inf")])
-def test_update_element_wrong_confidence(mock_elements_worker, confidence):
+@pytest.mark.parametrize(
+    "payload, error",
+    (
+        # Image
+        ({"image": "lol"}, "image should be a UUID"),
+        ({"image": 1234}, "image should be a UUID"),
+        ({"image": None}, "image should be a UUID"),
+    ),
+)
+def test_partial_update_element_wrong_param_image(mock_elements_worker, payload, error):
+    api_payload = {
+        "element": Element({"zone": None}),
+        **payload,
+    }
+
     with pytest.raises(AssertionError) as e:
-        mock_elements_worker.update_element(
-            element=Element({"zone": None}),
-            confidence=confidence,
+        mock_elements_worker.partial_update_element(
+            **api_payload,
         )
-    assert str(e.value) == "confidence should be None or a float in [0..1] range"
+    assert str(e.value) == error
 
 
-def test_update_element_api_error(responses, mock_elements_worker):
+def test_partial_update_element_api_error(responses, mock_elements_worker):
     elt = Element({"id": "12341234-1234-1234-1234-123412341234"})
     responses.add(
         responses.PATCH,
@@ -1306,7 +1420,7 @@ def test_update_element_api_error(responses, mock_elements_worker):
     )
 
     with pytest.raises(ErrorResponse):
-        mock_elements_worker.update_element(
+        mock_elements_worker.partial_update_element(
             element=elt,
             type="something",
             name="0",
@@ -1326,26 +1440,56 @@ def test_update_element_api_error(responses, mock_elements_worker):
     ]
 
 
-def test_update_element(
-    responses, mock_elements_worker_with_cache, mock_cached_elements
+@pytest.mark.parametrize(
+    "payload",
+    (
+        (
+            {
+                "polygon": [[10, 10], [20, 20], [20, 10], [10, 20]],
+                "confidence": None,
+            }
+        ),
+        (
+            {
+                "rotation_angle": 45,
+                "mirrored": False,
+            }
+        ),
+        (
+            {
+                "polygon": [[10, 10], [20, 20], [20, 10], [10, 20]],
+                "confidence": None,
+                "rotation_angle": 45,
+                "mirrored": False,
+            }
+        ),
+    ),
+)
+def test_partial_update_element(
+    responses,
+    mock_elements_worker_with_cache,
+    mock_cached_elements,
+    mock_cached_images,
+    payload,
 ):
     elt = CachedElement.select().first()
+    new_image = CachedImage.select().first()
+
     elt_response = {
-        "type": "new type",
-        "name": "new name",
-        "polygon": [[10, 10], [20, 20], [20, 10], [10, 20]],
-        "confidence": None,
+        "image": str(new_image.id),
+        **payload,
     }
     responses.add(
         responses.PATCH,
         f"http://testserver/api/v1/element/{elt.id}/",
         status=200,
+        # UUID not allowed in JSON
         json=elt_response,
     )
 
-    element_update_response = mock_elements_worker_with_cache.update_element(
+    element_update_response = mock_elements_worker_with_cache.partial_update_element(
         element=elt,
-        **elt_response,
+        **{**elt_response, "image": new_image.id},
     )
 
     assert len(responses.calls) == len(BASE_API_CALLS) + 1
@@ -1361,20 +1505,25 @@ def test_update_element(
     assert element_update_response == elt_response
 
     cached_element = CachedElement.get(CachedElement.id == elt.id)
-    assert cached_element.type == elt_response["type"]
-    assert cached_element.polygon == str(elt_response["polygon"])
-    assert cached_element.confidence == elt_response["confidence"]
+    # Always present in payload
+    assert str(cached_element.image_id) == elt_response["image"]
+    # Optional params
+    if "polygon" in payload:
+        # Cast to string as this is the only difference compared to model
+        elt_response["polygon"] = str(elt_response["polygon"])
+
+    for param in payload:
+        assert getattr(cached_element, param) == elt_response[param]
 
 
-def test_update_element_confidence(
-    responses, mock_elements_worker_with_cache, mock_cached_elements
+@pytest.mark.parametrize("confidence", (None, 0.42))
+def test_partial_update_element_confidence(
+    responses, mock_elements_worker_with_cache, mock_cached_elements, confidence
 ):
     elt = CachedElement.select().first()
     elt_response = {
-        "type": "new type",
-        "name": "new name",
         "polygon": [[10, 10], [20, 20], [20, 10], [10, 20]],
-        "confidence": 0.42,
+        "confidence": confidence,
     }
     responses.add(
         responses.PATCH,
@@ -1383,7 +1532,7 @@ def test_update_element_confidence(
         json=elt_response,
     )
 
-    element_update_response = mock_elements_worker_with_cache.update_element(
+    element_update_response = mock_elements_worker_with_cache.partial_update_element(
         element=elt,
         **elt_response,
     )
@@ -1401,9 +1550,8 @@ def test_update_element_confidence(
     assert element_update_response == elt_response
 
     cached_element = CachedElement.get(CachedElement.id == elt.id)
-    assert cached_element.type == elt_response["type"]
     assert cached_element.polygon == str(elt_response["polygon"])
-    assert cached_element.confidence == elt_response["confidence"]
+    assert cached_element.confidence == confidence
 
 
 def test_list_element_children_wrong_element(mock_elements_worker):

worker-{{cookiecutter.slug}}/Dockerfile

-FROM python:3
+FROM python:3.11
 
 WORKDIR /src