Refactored sidecar code

README.md

@@ -301,23 +301,33 @@ Options:
                                   the primary photo will be exported--
                                   associated burst images will be skipped.
   --sidecar FORMAT                Create sidecar for each photo exported;
-                                  valid FORMAT values: xmp, json; --sidecar
+                                  valid FORMAT values: xmp, json, exiftool;
-                                  json: create JSON sidecar useable by
+                                  --sidecar xmp: create XMP sidecar used by
-                                  exiftool (https://exiftool.org/) The sidecar
+                                  Adobe Lightroom, etc. The sidecar file is
-                                  file can be used to apply metadata to the
+                                  named in format photoname.ext.xmp The XMP
-                                  file with exiftool, for example: "exiftool
+                                  sidecar exports the following tags:
+                                  Description, Title, Keywords/Tags, Subject
+                                  (set to Keywords + PersonInImage),
+                                  PersonInImage, CreateDate, ModifyDate,
+                                  GPSLongitude. 
+                                  --sidecar json: create JSON
+                                  sidecar useable by exiftool
+                                  (https://exiftool.org/) The sidecar file can
+                                  be used to apply metadata to the file with
+                                  exiftool, for example: "exiftool
                                   -j=photoname.jpg.json photoname.jpg" The
                                   sidecar file is named in format
-                                  photoname.ext.json  --sidecar xmp: create
+                                  photoname.ext.json; format includes tag
-                                  XMP sidecar used by Adobe Lightroom, etc.The
+                                  groups (equivalent to running 'exiftool -G
-                                  sidecar file is named in format
+                                  -j'). 
-                                  photoname.ext.xmpThe XMP sidecar exports the
+                                  --sidecar exiftool: create JSON
-                                  following tags: Description, Title,
+                                  sidecar compatible with output of 'exiftool
-                                  Keywords/Tags, Subject (set to Keywords +
+                                  -j'. Unlike '--sidecar json', '--sidecar
-                                  PersonInImage), PersonInImage, CreateDate,
+                                  exiftool' does not export tag groups.
-                                  ModifyDate, GPSLongitude. For a list of tags
+                                  Sidecar filename is in format
-                                  exported in the JSON sidecar, see
+                                  photoname.ext.json; For a list of tags
-                                  --exiftool.
+                                  exported in the JSON and exiftool sidecar,
+                                  see '--exiftool'.
   --exiftool                      Use exiftool to write metadata directly to
                                   exported photos. To use this option,
                                   exiftool must be installed and in the path.
@@ -327,14 +337,12 @@ Options:
                                   metadata: EXIF:ImageDescription,
                                   XMP:Description (see also --description-
                                   template); XMP:Title; XMP:TagsList,
-                                  IPTC:Keywords (see also --keyword-template,
+                                  IPTC:Keywords, XMP:Subject (see also
-                                  --person-keyword, --album-keyword);
+                                  --keyword-template, --person-keyword,
-                                  XMP:Subject (set to keywords + person in
+                                  --album-keyword); XMP:PersonInImage;
-                                  image to mirror Photos' behavior);
+                                  EXIF:GPSLatitudeRef; EXIF:GPSLongitudeRef;
-                                  XMP:PersonInImage; EXIF:GPSLatitudeRef;
+                                  EXIF:GPSLatitude; EXIF:GPSLongitude;
-                                  EXIF:GPSLongitudeRef; EXIF:GPSLatitude;
+                                  EXIF:GPSPosition; EXIF:DateTimeOriginal;
-                                  EXIF:GPSLongitude; EXIF:GPSPosition;
-                                  EXIF:DateTimeOriginal;
                                   EXIF:OffsetTimeOriginal; EXIF:ModifyDate
                                   (see --ignore-date-modified);
                                   IPTC:DateCreated; IPTC:TimeCreated; (video

osxphotos/__main__.py

@@ -1342,8 +1342,8 @@ def query(
     metavar="FORMAT",
     type=click.Choice(["xmp", "json", "exiftool"], case_sensitive=False),
     help="Create sidecar for each photo exported; valid FORMAT values: xmp, json, exiftool; "
-    "--sidecar xmp: create XMP sidecar used by Adobe Lightroom, etc."
+    "--sidecar xmp: create XMP sidecar used by Adobe Lightroom, etc. "
-    "The sidecar file is named in format photoname.ext.xmp"
+    "The sidecar file is named in format photoname.ext.xmp "
     "The XMP sidecar exports the following tags: Description, Title, Keywords/Tags, "
     "Subject (set to Keywords + PersonInImage), PersonInImage, CreateDate, ModifyDate, "
     "GPSLongitude. "
@@ -1353,9 +1353,9 @@ def query(
     "The sidecar file is named in format photoname.ext.json; "
     "format includes tag groups (equivalent to running 'exiftool -G -j'). "
     "\n--sidecar exiftool: create JSON sidecar compatible with output of 'exiftool -j'. "
-    "Unlike --sidecar json, --sidecar exiftool does not export tag groups. "
+    "Unlike '--sidecar json', '--sidecar exiftool' does not export tag groups. "
-    "Sidecar filename is in format photoname.ext.json;"
+    "Sidecar filename is in format photoname.ext.json; "
-    "For a list of tags exported in the JSON sidecar, see --exiftool.",
+    "For a list of tags exported in the JSON and exiftool sidecar, see '--exiftool'.",
 )
 @click.option(
     "--exiftool",
@@ -1365,8 +1365,8 @@ def query(
     "exiftool may be installed from https://exiftool.org/.  "
     "Cannot be used with --export-as-hardlink.  Writes the following metadata: "
     "EXIF:ImageDescription, XMP:Description (see also --description-template); "
-    "XMP:Title; XMP:TagsList, IPTC:Keywords (see also --keyword-template, --person-keyword, --album-keyword); "
+    "XMP:Title; XMP:TagsList, IPTC:Keywords, XMP:Subject "
-    "XMP:Subject (set to keywords + person in image to mirror Photos' behavior); "
+    "(see also --keyword-template, --person-keyword, --album-keyword); " 
     "XMP:PersonInImage; EXIF:GPSLatitudeRef; EXIF:GPSLongitudeRef; EXIF:GPSLatitude; EXIF:GPSLongitude; "
     "EXIF:GPSPosition; EXIF:DateTimeOriginal; EXIF:OffsetTimeOriginal; "
     "EXIF:ModifyDate (see --ignore-date-modified); IPTC:DateCreated; IPTC:TimeCreated; "
@@ -1752,6 +1752,7 @@ def export(
 
     verbose_(f"osxphotos version {__version__}")
 
+    # validate options
     exclusive_options = [
         ("favorite", "not_favorite"),
         ("hidden", "not_hidden"),
@@ -1795,6 +1796,16 @@ def export(
         )
         raise click.Abort()
 
+    if all(x in [s.lower() for s in sidecar] for x in ["json", "exiftool"]):
+        click.echo(
+            click.style(
+                "Cannot use --sidecar json with --sidecar exiftool due to name collisions",
+                fg=CLI_COLOR_ERROR,
+            ),
+            err=True,
+        )
+        raise click.Abort()
+
     if save_config:
         verbose_(f"Saving options to file {save_config}")
         cfg.write_to_file(save_config)
@@ -3130,6 +3141,7 @@ def write_export_report(report_file, results):
             "converted_to_jpeg": 0,
             "sidecar_xmp": 0,
             "sidecar_json": 0,
+            "sidecar_exiftool": 0,
             "missing": 0,
             "error": 0,
             "exiftool_warning": "",
@@ -3175,6 +3187,14 @@ def write_export_report(report_file, results):
         all_results[result]["sidecar_json"] = 1
         all_results[result]["skipped"] = 1
 
+    for result in results.sidecar_exiftool_written:
+        all_results[result]["sidecar_exiftool"] = 1
+        all_results[result]["exported"] = 1
+
+    for result in results.sidecar_exiftool_skipped:
+        all_results[result]["sidecar_exiftool"] = 1
+        all_results[result]["skipped"] = 1
+
     for result in results.missing:
         all_results[result]["missing"] = 1
 
@@ -3198,6 +3218,7 @@ def write_export_report(report_file, results):
         "converted_to_jpeg",
         "sidecar_xmp",
         "sidecar_json",
+        "sidecar_exiftool",
         "missing",
         "error",
         "exiftool_warning",

osxphotos/photoinfo/_photoinfo_export.py

@@ -33,8 +33,8 @@ from .._constants import (
     _TEMPLATE_DIR,
     _UNKNOWN_PERSON,
     _XMP_TEMPLATE_NAME,
-    SIDECAR_JSON,
     SIDECAR_EXIFTOOL,
+    SIDECAR_JSON,
     SIDECAR_XMP,
 )
 from ..datetime_utils import datetime_tz_to_utc
@@ -44,8 +44,8 @@ from ..fileutil import FileUtil
 from ..photokit import (
     PHOTOS_VERSION_CURRENT,
     PHOTOS_VERSION_ORIGINAL,
-    PhotoLibrary,
     PhotoKitFetchFailed,
+    PhotoLibrary,
 )
 from ..utils import dd_to_dms_str, findfiles, noop
 
@@ -885,9 +885,14 @@ def export2(
             )
 
     # export metadata
-    # TODO: repetitive code here is prime for refactoring
+    sidecars = []
     sidecar_json_files_skipped = []
     sidecar_json_files_written = []
+    sidecar_exiftool_files_skipped = []
+    sidecar_exiftool_files_written = []
+    sidecar_xmp_files_skipped = []
+    sidecar_xmp_files_written = []
+
     if sidecar & SIDECAR_JSON:
         sidecar_filename = dest.parent / pathlib.Path(f"{dest.stem}{dest.suffix}.json")
         sidecar_str = self._exiftool_json_sidecar(
@@ -897,35 +902,16 @@ def export2(
             description_template=description_template,
             ignore_date_modified=ignore_date_modified,
         )
-        sidecar_digest = hexdigest(sidecar_str)
+        sidecars.append(
-        old_sidecar_digest, sidecar_sig = export_db.get_sidecar_for_file(
+            (
-            sidecar_filename
-        )
-        write_sidecar = (
-            not update
-            or (update and not sidecar_filename.exists())
-            or (
-                update
-                and (sidecar_digest != old_sidecar_digest)
-                or not fileutil.cmp_file_sig(sidecar_filename, sidecar_sig)
-            )
-        )
-        if write_sidecar:
-            verbose(f"Writing JSON sidecar {sidecar_filename}")
-            sidecar_json_files_written.append(str(sidecar_filename))
-            if not dry_run:
-                self._write_sidecar(sidecar_filename, sidecar_str)
-                export_db.set_sidecar_for_file(
                 sidecar_filename,
-                    sidecar_digest,
+                sidecar_str,
-                    fileutil.file_sig(sidecar_filename),
+                sidecar_json_files_written,
+                sidecar_json_files_skipped,
+                "JSON",
+            )
         )
-        else:
-            verbose(f"Skipped up to date JSON sidecar {sidecar_filename}")
-            sidecar_json_files_skipped.append(str(sidecar_filename))
 
-    sidecar_exiftool_files_skipped = []
-    sidecar_exiftool_files_written = []
     if sidecar & SIDECAR_EXIFTOOL:
         sidecar_filename = dest.parent / pathlib.Path(f"{dest.stem}{dest.suffix}.json")
         sidecar_str = self._exiftool_json_sidecar(
@@ -936,35 +922,16 @@ def export2(
             ignore_date_modified=ignore_date_modified,
             tag_groups=False,
         )
-        sidecar_digest = hexdigest(sidecar_str)
+        sidecars.append(
-        old_sidecar_digest, sidecar_sig = export_db.get_sidecar_for_file(
+            (
-            sidecar_filename
-        )
-        write_sidecar = (
-            not update
-            or (update and not sidecar_filename.exists())
-            or (
-                update
-                and (sidecar_digest != old_sidecar_digest)
-                or not fileutil.cmp_file_sig(sidecar_filename, sidecar_sig)
-            )
-        )
-        if write_sidecar:
-            verbose(f"Writing exiftool sidecar {sidecar_filename}")
-            sidecar_exiftool_files_written.append(str(sidecar_filename))
-            if not dry_run:
-                self._write_sidecar(sidecar_filename, sidecar_str)
-                export_db.set_sidecar_for_file(
                 sidecar_filename,
-                    sidecar_digest,
+                sidecar_str,
-                    fileutil.file_sig(sidecar_filename),
+                sidecar_exiftool_files_written,
+                sidecar_exiftool_files_skipped,
+                "exiftool",
+            )
         )
-        else:
-            verbose(f"Skipped up to date exiftool sidecar {sidecar_filename}")
-            sidecar_exiftool_files_skipped.append(str(sidecar_filename))
 
-    sidecar_xmp_files_skipped = []
-    sidecar_xmp_files_written = []
     if sidecar & SIDECAR_XMP:
         sidecar_filename = dest.parent / pathlib.Path(f"{dest.stem}{dest.suffix}.xmp")
         sidecar_str = self._xmp_sidecar(
@@ -974,6 +941,23 @@ def export2(
             description_template=description_template,
             extension=dest.suffix[1:] if dest.suffix else None,
         )
+        sidecars.append(
+            (
+                sidecar_filename,
+                sidecar_str,
+                sidecar_xmp_files_written,
+                sidecar_xmp_files_skipped,
+                "XMP",
+            )
+        )
+
+    for data in sidecars:
+        sidecar_filename = data[0]
+        sidecar_str = data[1]
+        files_written = data[2]
+        files_skipped = data[3]
+        sidecar_type = data[4]
+
         sidecar_digest = hexdigest(sidecar_str)
         old_sidecar_digest, sidecar_sig = export_db.get_sidecar_for_file(
             sidecar_filename
@@ -988,8 +972,8 @@ def export2(
             )
         )
         if write_sidecar:
-            verbose(f"Writing XMP sidecar {sidecar_filename}")
+            verbose(f"Writing {sidecar_type} sidecar {sidecar_filename}")
-            sidecar_xmp_files_written.append(str(sidecar_filename))
+            files_written.append(str(sidecar_filename))
             if not dry_run:
                 self._write_sidecar(sidecar_filename, sidecar_str)
                 export_db.set_sidecar_for_file(
@@ -998,8 +982,8 @@ def export2(
                     fileutil.file_sig(sidecar_filename),
                 )
         else:
-            verbose(f"Skipped up to date XMP sidecar {sidecar_filename}")
+            verbose(f"Skipped up to date {sidecar_type} sidecar {sidecar_filename}")
-            sidecar_xmp_files_skipped.append(str(sidecar_filename))
+            files_skipped.append(str(sidecar_filename))
 
     # if exiftool, write the metadata
     if update:

tests/test_cli.py

@@ -2256,6 +2256,33 @@ def test_export_sidecar_update():
         assert "Writing JSON sidecar" in result.output
 
 
+def test_export_sidecar_invalid():
+    """ test invalid combination of sidecars """
+    import os
+
+    from osxphotos.__main__ import cli
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            cli,
+            [
+                "export",
+                "--db",
+                os.path.join(cwd, CLI_PHOTOS_DB),
+                ".",
+                "--sidecar=json",
+                "--sidecar=exiftool",
+                f"--uuid={CLI_EXPORT_UUID}",
+                "-V",
+            ],
+        )
+        assert result.exit_code != 0
+        assert "Cannot use --sidecar json with --sidecar exiftool" in result.output
+
+
 def test_export_live():
     import glob
     import os