From ade98fc15051684bfb54d0199d9c370481b70dcc Mon Sep 17 00:00:00 2001 From: Rhet Turnbull Date: Mon, 28 Dec 2020 08:23:23 -0800 Subject: [PATCH] Refactored sidecar code --- README.md | 54 +++++++----- osxphotos/__main__.py | 35 ++++++-- osxphotos/photoinfo/_photoinfo_export.py | 104 ++++++++++------------- tests/test_cli.py | 27 ++++++ 4 files changed, 130 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index bb9c2600..289b921c 100644 --- a/README.md +++ b/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 - 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 + valid FORMAT values: xmp, json, exiftool; + --sidecar xmp: create XMP sidecar used by + Adobe Lightroom, etc. 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. + --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 - XMP sidecar used by Adobe Lightroom, etc.The - sidecar file is named in format - photoname.ext.xmpThe XMP sidecar exports the - following tags: Description, Title, - Keywords/Tags, Subject (set to Keywords + - PersonInImage), PersonInImage, CreateDate, - ModifyDate, GPSLongitude. For a list of tags - exported in the JSON sidecar, see - --exiftool. + photoname.ext.json; format includes tag + groups (equivalent to running 'exiftool -G + -j'). + --sidecar exiftool: create JSON + sidecar compatible with output of 'exiftool + -j'. Unlike '--sidecar json', '--sidecar + exiftool' does not export tag groups. + Sidecar filename is in format + photoname.ext.json; For a list of tags + 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, - --person-keyword, --album-keyword); - XMP:Subject (set to keywords + person in - image to mirror Photos' behavior); - XMP:PersonInImage; EXIF:GPSLatitudeRef; - EXIF:GPSLongitudeRef; EXIF:GPSLatitude; - EXIF:GPSLongitude; EXIF:GPSPosition; - EXIF:DateTimeOriginal; + IPTC:Keywords, XMP:Subject (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; (video diff --git a/osxphotos/__main__.py b/osxphotos/__main__.py index 4a7065f8..97083267 100644 --- a/osxphotos/__main__.py +++ b/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." - "The sidecar file is named in format photoname.ext.xmp" + "--sidecar xmp: create XMP sidecar used by Adobe Lightroom, etc. " + "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. " - "Sidecar filename is in format photoname.ext.json;" - "For a list of tags exported in the JSON sidecar, see --exiftool.", + "Unlike '--sidecar json', '--sidecar exiftool' does not export tag groups. " + "Sidecar filename is in format photoname.ext.json; " + "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:Subject (set to keywords + person in image to mirror Photos' behavior); " + "XMP:Title; XMP:TagsList, IPTC:Keywords, XMP:Subject " + "(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", diff --git a/osxphotos/photoinfo/_photoinfo_export.py b/osxphotos/photoinfo/_photoinfo_export.py index d615c9d7..48a89f63 100644 --- a/osxphotos/photoinfo/_photoinfo_export.py +++ b/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 @@ -396,7 +396,7 @@ def export( sidecar |= SIDECAR_EXIFTOOL if sidecar_xmp: sidecar |= SIDECAR_XMP - + results = self.export2( dest, *filename, @@ -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) - 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) + sidecars.append( + ( + sidecar_filename, + sidecar_str, + sidecar_json_files_written, + sidecar_json_files_skipped, + "JSON", ) ) - 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, - fileutil.file_sig(sidecar_filename), - ) - 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) - 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) + sidecars.append( + ( + sidecar_filename, + sidecar_str, + sidecar_exiftool_files_written, + sidecar_exiftool_files_skipped, + "exiftool", ) ) - 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, - fileutil.file_sig(sidecar_filename), - ) - 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}") - sidecar_xmp_files_written.append(str(sidecar_filename)) + verbose(f"Writing {sidecar_type} sidecar {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}") - sidecar_xmp_files_skipped.append(str(sidecar_filename)) + verbose(f"Skipped up to date {sidecar_type} sidecar {sidecar_filename}") + files_skipped.append(str(sidecar_filename)) # if exiftool, write the metadata if update: diff --git a/tests/test_cli.py b/tests/test_cli.py index 29b44071..24d33679 100644 --- a/tests/test_cli.py +++ b/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