metadata writer: some improvements, especially tests

src/photools3/metadata_writer.py

@@ -23,7 +23,7 @@ SIDECAR_EXTENSIONS = {".dng", ".nef", ".cr2", ".cr3", ".arw", ".raf", ".orf", ".
 XmpKey = Annotated[str, StringConstraints(pattern=r"^\w+\:\w+")]
 
 type Caption = str
-type Keywords = list[str] | None
+type Keywords = list[str]
 type Color = str | None
 type XmpMetadata = dict[str, Any]
 
@@ -70,44 +70,66 @@ class MetadataWriter:
     def sidecar_path(self) -> Path:
         return Path(str(self.path) + ".xmp")
 
-    def get_metadata(self, *args, **kwargs) -> Any:
-        path = self.sidecar_path
+    @property
+    def metadata(self) -> Any:
+        path = self.sidecar_path if self.sidecar_path.exists() else self.path
         if not path.exists():
-            return ""
+            log.warning(f"File not found: {path}")
+            return None
         try:
             with exiftool.ExifToolHelper() as exif:
                 metadata = exif.get_metadata([path])[0]
         except Exception as exc:  # noqa: BLE001
             log.error(f"Could not read metadata from {path}: {exc}")
-            return ""
-        return metadata.get(*args, **kwargs)
+            return None
+        return metadata
 
-    def set_metadata(self, metadata_tags: dict[str, Any]) -> bool:
-        if self.sidecar_path.exists():
-            files, params = [str(self.sidecar_path)], ["-overwrite_original"]
-        else:
-            files, params = [str(self.path)], ["-o", str(self.sidecar_path)]
+    def get_caption(self) -> str:
+        result = self.metadata.get(XMP_KEYS_CAPTION[0], "")
+        assert isinstance(result, str)
+        return result
+
+    def get_keywords(self) -> list[str]:
+        result = self.metadata.get(XMP_KEYS_KEYWORDS[0], []) or []
+        if isinstance(result, str):
+            result = [result]
+        assert isinstance(result, list)
+        return result
+
+    def set_metadata(self, metadata_tags: dict[str, Any]):
         try:
             with exiftool.ExifToolHelper() as exif:
-                exif.set_tags(files, tags=metadata_tags, params=params)
+                if not self.sidecar_path.exists():
+                    exif.execute("-o", str(self.sidecar_path), str(self.path))
+                exif.set_tags(
+                    [str(self.sidecar_path)],
+                    tags=metadata_tags,
+                    params=["-overwrite_original"],
+                )
         except Exception as exc:  # noqa: BLE001
             log.error(f"Could not write metadata to file {self.sidecar_path}: {exc}")
-            return False
-        return True
 
-    def write_caption(self, caption: Caption):
-        existing = self.get_metadata(XMP_KEYS_CAPTION[0], "")
+    def write_caption(
+        self, caption: Caption, include_prefix: bool = True, override: bool = True
+    ):
+        if include_prefix:
+            caption = _prefixed(caption)
+        existing = self.get_caption()
         is_existing_prefixed = _is_prefixed(existing)
-        if existing and is_existing_prefixed:
+        if existing and is_existing_prefixed and not override:
             return
         elif existing and not is_existing_prefixed:
             caption = f"{existing}. {_prefixed(caption)}"
         metadata_tags = {key: caption for key in XMP_KEYS_CAPTION}
         self.set_metadata(metadata_tags)
 
-    def write_keywords(self, keywords: Keywords):
-        # TODO
-        ...
+    def write_keywords(self, keywords: Keywords, include_prefix: bool = True):
+        if include_prefix:
+            keywords = list(map(_prefixed, keywords))
+        existing = self.get_keywords()
+        keywords = list(set(existing + keywords))
+        metadata_tags = {tag: keywords for tag in XMP_KEYS_KEYWORDS}
+        self.set_metadata(metadata_tags)
 
     def write_color(self, color: Color):
         # TODO
@@ -116,7 +138,7 @@ class MetadataWriter:
     def write(
         self,
         caption: Caption | None = None,
-        keywords: Keywords = None,
+        keywords: Keywords | None = None,
         color: Color = None,
     ):
         if caption is not None:

test/test_metadata_writer.py

@@ -4,44 +4,61 @@ from pathlib import Path
 import pytest
 
 from photools3.metadata_writer import (
-    XMP_KEYS_CAPTION,
     MetadataWriter,
+    _is_prefixed,
+    _prefixed,
 )
 from test.test_photo import PATHS_DNG_EXAMPLES
 
 
 @pytest.fixture
 def make_dng_file(tmp_path):
-    def _make(index: int = 0) -> Path:
-        src = PATHS_DNG_EXAMPLES[index]
-        dst = tmp_path / f"copy_{index}_{src.name}"
+    def _make(src: Path) -> Path:
+        dst = tmp_path / f"copy_{src.name}"
         shutil.copy(src, dst)
         return dst
 
     return _make
 
 
-def test_read_metadata(make_dng_file):
+@pytest.mark.parametrize("pic", PATHS_DNG_EXAMPLES)
+def test_read_metadata(make_dng_file, pic):
     """primitive test for `MetadataWriter`"""
-    file_path = make_dng_file()
+    file_path = make_dng_file(pic)
     result = MetadataWriter(file_path)
     assert result
 
 
-@pytest.mark.parametrize("pic_no", range(len(PATHS_DNG_EXAMPLES)))
-def test_read_metadata_sidecar_path(make_dng_file, pic_no):
-    file_path = make_dng_file(pic_no)
+def test_prefixed():
+    txt = "bla_bla"
+    txt_prefixed = _prefixed(txt)
+    assert txt_prefixed == "photools: bla_bla"
+
+
+def test_is_prefixed():
+    txt_prefixed = "photools: some text"
+    txt_not_prefixed = "prefix is missing here"
+    txt_false_prefix = "some_perfix: but not the right one"
+
+    assert _is_prefixed(txt_prefixed)
+    assert not _is_prefixed(txt_not_prefixed)
+    assert not _is_prefixed(txt_false_prefix)
+
+
+@pytest.mark.parametrize("pic", PATHS_DNG_EXAMPLES)
+def test_read_metadata_sidecar_path(make_dng_file, pic):
+    file_path = make_dng_file(pic)
     metadata_writer = MetadataWriter(file_path)
     print(metadata_writer.sidecar_path)
     assert isinstance(metadata_writer.sidecar_path, Path)
     assert str(metadata_writer.sidecar_path).lower().endswith(".dng.xmp")
 
 
-@pytest.mark.parametrize("pic_no", range(len(PATHS_DNG_EXAMPLES)))
-def test_set_caption(make_dng_file, pic_no):
-    metadata_writer = MetadataWriter(make_dng_file(pic_no))
+@pytest.mark.parametrize("pic", PATHS_DNG_EXAMPLES)
+def test_set_caption(make_dng_file, pic):
+    metadata_writer = MetadataWriter(make_dng_file(pic))
     metadata_writer.write_caption("This is a test")
-    result = metadata_writer.get_metadata(XMP_KEYS_CAPTION[0])
+    result = metadata_writer.get_caption()
     assert result
 
 
@@ -50,9 +67,16 @@ def test_set_caption_on_existing():
     ...
 
 
-def test_set_keywords():
-    # TODO
-    ...
+@pytest.mark.parametrize("pic", PATHS_DNG_EXAMPLES)
+def test_set_keywords(make_dng_file, pic):
+    metadata_writer = MetadataWriter(make_dng_file(pic))
+    metadata_writer.write_keywords(["hund", "katze", "maus"])
+    result = metadata_writer.get_keywords()
+    assert result
+    assert isinstance(result, list)
+    assert "photools: hund" in result
+    assert "photools: katze" in result
+    assert "photools: maus" in result
 
 
 def test_set_keywords_on_existing():