remove duplicate keywords with --exiftool and --sidecar, closes #294

Documentation fix for #293. Thanks to @finestream

.all-contributorsrc

@@ -109,6 +109,15 @@
       "contributions": [
         "code"
       ]
+    },
+    {
+      "login": "finestream",
+      "name": "finestream",
+      "avatar_url": "https://avatars1.githubusercontent.com/u/16638513?v=4",
+      "profile": "https://github.com/finestream",
+      "contributions": [
+        "doc"
+      ]
     }
   ],
   "contributorsPerLine": 7

CHANGELOG.md

@@ -4,6 +4,35 @@ All notable changes to this project will be documented in this file. Dates are d
 
 Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
 
+#### [v0.38.7](https://github.com/RhetTbull/osxphotos/compare/v0.38.6...v0.38.7)
+
+> 21 December 2020
+
+- Added better exiftool error handling, closes #300 [`#300`](https://github.com/RhetTbull/osxphotos/issues/300)
+- README.md updates for tested versions [`8d1ccda`](https://github.com/RhetTbull/osxphotos/commit/8d1ccda0c897f84342caf612c1070d78bff421f5)
+- version bump [`ef94933`](https://github.com/RhetTbull/osxphotos/commit/ef94933dd87b9ad2a516163ca50a36753dacd55a)
+
+#### [v0.38.6](https://github.com/RhetTbull/osxphotos/compare/v0.38.5...v0.38.6)
+
+> 18 December 2020
+
+- Documentation fix for #293. Thanks to @finestream  [`#295`](https://github.com/RhetTbull/osxphotos/pull/295)
+- Added additional test cases for #286, --ignore-signature [`880a9b6`](https://github.com/RhetTbull/osxphotos/commit/880a9b67a14787ef23ae68ad3164d7eda1af16ec)
+-  Add @finestream as a contributor [`ad860b1`](https://github.com/RhetTbull/osxphotos/commit/ad860b1500dffd846322e05562ba4f2019cd1017)
+- Fixed issue #296 [`a7c688c`](https://github.com/RhetTbull/osxphotos/commit/a7c688cfc2221833e0252d71bbe596eee5f9a6e8)
+- Updated README.md [`d40b16a`](https://github.com/RhetTbull/osxphotos/commit/d40b16a456c64014674505b7c715c80b977da76a)
+- Version bump [`4678f15`](https://github.com/RhetTbull/osxphotos/commit/4678f15bc86b5dedcb73c73f40e5fe11c1b51fa0)
+
+#### [v0.38.5](https://github.com/RhetTbull/osxphotos/compare/v0.38.4...v0.38.5)
+
+> 17 December 2020
+
+- Patch 1 [`#1`](https://github.com/RhetTbull/osxphotos/pull/1)
+- Implemented --ignore-signature, issue #286 [`e394d8e`](https://github.com/RhetTbull/osxphotos/commit/e394d8e6be7607a1668029bcb37ccb30a4fa792f)
+- Update __main__.py [`e097f3a`](https://github.com/RhetTbull/osxphotos/commit/e097f3aad546b5be5eabab529bd2c35ce3056876)
+- Update README.md [`4f64eeb`](https://github.com/RhetTbull/osxphotos/commit/4f64eeb996d43953eb90618465d2bd046282c4bb)
+- Update README.md [`3155045`](https://github.com/RhetTbull/osxphotos/commit/3155045ec87d83285f2e66210559f4be0a10e3a2)
+
 #### [v0.38.4](https://github.com/RhetTbull/osxphotos/compare/v0.38.3...v0.38.4)
 
 > 14 December 2020

README.md

@@ -3,7 +3,7 @@
 [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT)
 [![Python package](https://github.com/RhetTbull/osxphotos/workflows/Python%20package/badge.svg)](https://github.com/RhetTbull/osxphotos/workflows/Python%20package/badge.svg)
 <!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->
-[![All Contributors](https://img.shields.io/badge/all_contributors-11-orange.svg?style=flat-square)](#contributors-)
+[![All Contributors](https://img.shields.io/badge/all_contributors-12-orange.svg?style=flat-square)](#contributors-)
 <!-- ALL-CONTRIBUTORS-BADGE:END -->
 
 - [OSXPhotos](#osxphotos)
@@ -43,13 +43,13 @@ OSXPhotos provides the ability to interact with and query Apple's Photos.app lib
 
 ## Supported operating systems
 
-Only works on MacOS (aka Mac OS X). Tested on MacOS 10.12.6 / Photos 2.0, 10.13.6 / Photos 3.0, MacOS 10.14.5, 10.14.6 / Photos 4.0, MacOS 10.15.1 - 10.15.6 / Photos 5.0.
+Only works on MacOS (aka Mac OS X). Tested on MacOS 10.12.6 / Photos 2.0, 10.13.6 / Photos 3.0, MacOS 10.14.5, 10.14.6 / Photos 4.0, MacOS 10.15.1 - 10.15.7 / Photos 5.0.
 
-Beta support for MacOS 10.16/MacOS 11 Big Sur Beta / Photos 6.0.
+Beta support for MacOS 10.16/MacOS 11 Big Sur Beta / Photos 6.0.  Not tested on M1 / Apple silicon Macs.
 
 Requires python >= 3.7. 
 
-This package will read Photos databases for any supported version on any supported OS version.  E.g. you can read a database created with Photos 4.0 on MacOS 10.14 on a machine running MacOS 10.12.
+This package will read Photos databases for any supported version on any supported OS version.  E.g. you can read a database created with Photos 5.0 on MacOS 10.15 on a machine running MacOS 10.12 and vice versa.
 
 
 ## Installation instructions
@@ -474,6 +474,10 @@ will be rendered to substitute template fields with values from the photo.
 For example, '{created.month}' would be replaced with the month name of the
 photo creation date.  e.g. 'November'.
 
+Some options supporting templates may be repeated e.g., --keyword-template
+'{label}'  --keyword-template '{media_type}' to add both labels and media
+types to the  keywords.
+
 The general format for a template is '{TEMPLATE_FIELD[,[DEFAULT]]}'.  Some
 templates have optional modifiers in form
 '{[[DELIM]+]TEMPLATE_FIELD[(PATH_SEP)][?VALUE_IF_TRUE][,[DEFAULT]]}'
@@ -741,6 +745,10 @@ Example: export photos to file structure based on 4-digit year and full name of
 
 `osxphotos export ~/Desktop/export --directory "{created.year}/{created.month}"`
 
+Example: export photos to file structure based on 4-digit year of photo's creation date and add keywords for media type and labels (labels are only awailable on Photos 5 and higher):
+
+`osxphotos export ~/Desktop/export --directory "{created.year}" --keyword-template "{label}" --keyword-template "{media_type}"` 
+
 Example: export default library using 'country name/year' as output directory (but use "NoCountry/year" if country not specified), add persons, album names, and year as keywords, write exif metadata to files when exporting, update only changed files, print verbose ouput
 
 `osxphotos export ~/Desktop/export --directory "{place.name.country,NoCountry}/{created.year}"  --person-keyword --album-keyword --keyword-template "{created.year}" --exiftool --update --verbose`
@@ -2242,6 +2250,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
     <td align="center"><a href="https://github.com/agprimatic"><img src="https://avatars1.githubusercontent.com/u/4685054?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Ag Primatic</b></sub></a><br /><a href="https://github.com/RhetTbull/osxphotos/commits?author=agprimatic" title="Code">💻</a></td>
     <td align="center"><a href="https://github.com/hhoeck"><img src="https://avatars1.githubusercontent.com/u/6313998?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Horst Höck</b></sub></a><br /><a href="https://github.com/RhetTbull/osxphotos/commits?author=hhoeck" title="Code">💻</a></td>
     <td align="center"><a href="https://github.com/jstrine"><img src="https://avatars1.githubusercontent.com/u/33943447?v=4?s=100" width="100px;" alt=""/><br /><sub><b>Jonathan Strine</b></sub></a><br /><a href="https://github.com/RhetTbull/osxphotos/commits?author=jstrine" title="Code">💻</a></td>
+    <td align="center"><a href="https://github.com/finestream"><img src="https://avatars1.githubusercontent.com/u/16638513?v=4?s=100" width="100px;" alt=""/><br /><sub><b>finestream</b></sub></a><br /><a href="https://github.com/RhetTbull/osxphotos/commits?author=finestream" title="Documentation">📖</a></td>
   </tr>
 </table>
 

osxphotos/__main__.py

@@ -19,6 +19,8 @@ from ._constants import (
     _EXIF_TOOL_URL,
     _PHOTOS_4_VERSION,
     _UNKNOWN_PLACE,
+    CLI_COLOR_ERROR,
+    CLI_COLOR_WARNING,
     DEFAULT_JPEG_QUALITY,
     DEFAULT_EDITED_SUFFIX,
     DEFAULT_ORIGINAL_SUFFIX,
@@ -50,7 +52,15 @@ OSXPHOTOS_EXPORT_DB = ".osxphotos_export.db"
 def verbose_(*args, **kwargs):
     """ print output if verbose flag set """
     if VERBOSE:
-        click.echo(*args, **kwargs)
+        styled_args = []
+        for arg in args:
+            if type(arg) == str:
+                if "error" in arg.lower():
+                    arg = click.style(arg, fg=CLI_COLOR_ERROR)
+                elif "warning" in arg.lower():
+                    arg = click.style(arg, fg=CLI_COLOR_WARNING)
+            styled_args.append(arg)
+        click.echo(*styled_args, **kwargs)
 
 
 def normalize_unicode(value):
@@ -177,6 +187,10 @@ which will be rendered to substitute template fields with values from the photo.
 For example, '{created.month}' would be replaced with the month name of the photo creation date. 
 e.g. 'November'. 
 \n
+Some options supporting templates may be repeated e.g., --keyword-template '{label}' 
+--keyword-template '{media_type}' to add both labels and media types to the 
+keywords.
+\n
 The general format for a template is '{TEMPLATE_FIELD[,[DEFAULT]]}'. 
 Some templates have optional modifiers in form 
 '{[[DELIM]+]TEMPLATE_FIELD[(PATH_SEP)][?VALUE_IF_TRUE][,[DEFAULT]]}'
@@ -1601,7 +1615,11 @@ def export(
             cfg.load_from_file(load_config)
         except ConfigOptionsLoadError as e:
             click.echo(
-                f"Error parsing {load_config} config file: {e.message}", err=True
+                click.style(
+                    f"Error parsing {load_config} config file: {e.message}",
+                    fg=CLI_COLOR_ERROR,
+                ),
+                err=True,
             )
             raise click.Abort()
 
@@ -1733,7 +1751,12 @@ def export(
     try:
         cfg.validate(exclusive=exclusive_options, dependent=dependent_options, cli=True)
     except ConfigOptionsInvalidError as e:
-        click.echo(f"Incompatible export options: {e.message}", err=True)
+        click.echo(
+            click.style(
+                f"Incompatible export options: {e.message}", fg=CLI_COLOR_ERROR
+            ),
+            err=True,
+        )
         raise click.Abort()
 
     if save_config:
@@ -1748,13 +1771,20 @@ def export(
     )
 
     if not os.path.isdir(dest):
-        click.echo(f"DEST {dest} must be valid path", err=True)
+        click.echo(
+            click.style(f"DEST {dest} must be valid path", fg=CLI_COLOR_ERROR), err=True
+        )
         raise click.Abort()
 
     dest = str(pathlib.Path(dest).resolve())
 
     if report and os.path.isdir(report):
-        click.echo(f"report is a directory, must be file name", err=True)
+        click.echo(
+            click.style(
+                f"report is a directory, must be file name", fg=CLI_COLOR_ERROR
+            ),
+            err=True,
+        )
         raise click.Abort()
 
     # if use_photokit and not check_photokit_authorization():
@@ -1781,8 +1811,11 @@ def export(
             _ = get_exiftool_path()
         except FileNotFoundError:
             click.echo(
-                "Could not find exiftool. Please download and install"
-                " from https://exiftool.org/",
+                click.style(
+                    "Could not find exiftool. Please download and install"
+                    " from https://exiftool.org/",
+                    fg=CLI_COLOR_ERROR,
+                ),
                 err=True,
             )
             ctx.exit(2)
@@ -1815,10 +1848,13 @@ def export(
     other_db_files = find_files_in_branch(dest, OSXPHOTOS_EXPORT_DB)
     if other_db_files:
         click.echo(
-            "WARNING: found other export database files in this destination directory branch.  "
-            + "This likely means you are attempting to export files into a directory "
-            + "that is either the parent or a child directory of a previous export. "
-            + "Proceeding may cause your exported files to be overwritten.",
+            click.style(
+                "WARNING: found other export database files in this destination directory branch.  "
+                + "This likely means you are attempting to export files into a directory "
+                + "that is either the parent or a child directory of a previous export. "
+                + "Proceeding may cause your exported files to be overwritten.",
+                fg=CLI_COLOR_WARNING,
+            ),
             err=True,
         )
         click.echo(
@@ -1925,22 +1961,10 @@ def export(
         # because the original code used --original-name as an option
         original_name = not current_name
 
-        results_exported = []
-        results_new = []
-        results_updated = []
-        results_skipped = []
-        results_exif_updated = []
-        results_touched = []
-        results_converted = []
-        results_sidecar_json_written = []
-        results_sidecar_json_skipped = []
-        results_sidecar_xmp_written = []
-        results_sidecar_xmp_skipped = []
-        results_missing = []
-        results_error = []
+        results = ExportResults()
         if verbose:
             for p in photos:
-                results = export_photo(
+                export_results = export_photo(
                     photo=p,
                     dest=dest,
                     verbose=verbose,
@@ -1975,19 +1999,7 @@ def export(
                     ignore_date_modified=ignore_date_modified,
                     use_photokit=use_photokit,
                 )
-                results_exported.extend(results.exported)
-                results_new.extend(results.new)
-                results_updated.extend(results.updated)
-                results_skipped.extend(results.skipped)
-                results_exif_updated.extend(results.exif_updated)
-                results_touched.extend(results.touched)
-                results_converted.extend(results.converted_to_jpeg)
-                results_sidecar_json_written.extend(results.sidecar_json_written)
-                results_sidecar_json_skipped.extend(results.sidecar_json_skipped)
-                results_sidecar_xmp_written.extend(results.sidecar_xmp_written)
-                results_sidecar_xmp_skipped.extend(results.sidecar_xmp_skipped)
-                results_missing.extend(results.missing)
-                results_error.extend(results.error)
+                results += export_results
 
                 # if convert_to_jpeg and p.isphoto and p.uti != "public.jpeg":
                 #     for photo_file in set(
@@ -1999,7 +2011,7 @@ def export(
             # show progress bar
             with click.progressbar(photos) as bar:
                 for p in bar:
-                    results = export_photo(
+                    export_results = export_photo(
                         photo=p,
                         dest=dest,
                         verbose=verbose,
@@ -2034,19 +2046,7 @@ def export(
                         ignore_date_modified=ignore_date_modified,
                         use_photokit=use_photokit,
                     )
-                    results_exported.extend(results.exported)
-                    results_new.extend(results.new)
-                    results_updated.extend(results.updated)
-                    results_skipped.extend(results.skipped)
-                    results_exif_updated.extend(results.exif_updated)
-                    results_touched.extend(results.touched)
-                    results_converted.extend(results.converted_to_jpeg)
-                    results_sidecar_json_written.extend(results.sidecar_json_written)
-                    results_sidecar_json_skipped.extend(results.sidecar_json_skipped)
-                    results_sidecar_xmp_written.extend(results.sidecar_xmp_written)
-                    results_sidecar_xmp_skipped.extend(results.sidecar_xmp_skipped)
-                    results_missing.extend(results.missing)
-                    results_error.extend(results.error)
+                    results += export_results
 
         # print summary results
         # print(f"results_exported: {results_exported}")
@@ -2065,19 +2065,19 @@ def export(
 
         if cleanup:
             all_files = (
-                results_exported
-                + results_skipped
-                + results_exif_updated
-                + results_touched
-                + results_converted
-                + results_sidecar_json_written
-                + results_sidecar_json_skipped
-                + results_sidecar_xmp_written
-                + results_sidecar_xmp_skipped
+                results.exported
+                + results.skipped
+                + results.exif_updated
+                + results.touched
+                + results.converted_to_jpeg
+                + results.sidecar_json_written
+                + results.sidecar_json_skipped
+                + results.sidecar_xmp_written
+                + results.sidecar_xmp_skipped
                 # include missing so a file that was already in export directory
                 # but was missing on --update doesn't get deleted
                 # (better to have old version than none)
-                + results_missing
+                + results.missing
                 + [str(pathlib.Path(export_db_path).resolve())]
             )
             click.echo(f"Cleaning up {dest}")
@@ -2088,41 +2088,26 @@ def export(
 
         if report:
             verbose_(f"Writing export report to {report}")
-            write_export_report(
-                report,
-                results_exported=results_exported,
-                results_new=results_new,
-                results_updated=results_updated,
-                results_skipped=results_skipped,
-                results_exif_updated=results_exif_updated,
-                results_touched=results_touched,
-                results_converted=results_converted,
-                results_sidecar_json_written=results_sidecar_json_written,
-                results_sidecar_json_skipped=results_sidecar_json_skipped,
-                results_sidecar_xmp_written=results_sidecar_xmp_written,
-                results_sidecar_xmp_skipped=results_sidecar_xmp_skipped,
-                results_missing=results_missing,
-                results_error=results_error,
-            )
+            write_export_report(report, results)
 
         photo_str_total = "photos" if len(photos) != 1 else "photo"
         if update:
             summary = (
                 f"Processed: {len(photos)} {photo_str_total}, "
-                f"exported: {len(results_new)}, "
-                f"updated: {len(results_updated)}, "
-                f"skipped: {len(results_skipped)}, "
-                f"updated EXIF data: {len(results_exif_updated)}, "
+                f"exported: {len(results.new)}, "
+                f"updated: {len(results.updated)}, "
+                f"skipped: {len(results.skipped)}, "
+                f"updated EXIF data: {len(results.exif_updated)}, "
             )
         else:
             summary = (
                 f"Processed: {len(photos)} {photo_str_total}, "
-                f"exported: {len(results_exported)}, "
+                f"exported: {len(results.exported)}, "
             )
-        summary += f"missing: {len(results_missing)}, "
-        summary += f"error: {len(results_error)}"
+        summary += f"missing: {len(results.missing)}, "
+        summary += f"error: {len(results.error)}"
         if touch_file:
-            summary += f", touched date: {len(results_touched)}"
+            summary += f", touched date: {len(results.touched)}"
         click.echo(summary)
         stop_time = time.perf_counter()
         click.echo(f"Elapsed time: {(stop_time-start_time):.3f} seconds")
@@ -2647,19 +2632,7 @@ def export_photo(
     global VERBOSE
     VERBOSE = bool(verbose)
 
-    results_exported = []
-    results_new = []
-    results_updated = []
-    results_skipped = []
-    results_exif_updated = []
-    results_touched = []
-    results_converted = []
-    results_sidecar_json_written = []
-    results_sidecar_json_skipped = []
-    results_sidecar_xmp_written = []
-    results_sidecar_xmp_skipped = []
-    results_error = []
-    results_missing = []
+    results = ExportResults()
 
     export_original = not (skip_original_if_edited and photo.hasadjustments)
 
@@ -2763,7 +2736,7 @@ def export_photo(
                 if missing_original:
                     space = " " if not verbose else ""
                     verbose_(f"{space}Skipping missing photo {photo.original_filename}")
-                    results_missing.append(
+                    results.missing.append(
                         str(pathlib.Path(dest_path) / original_filename)
                     )
                 else:
@@ -2795,33 +2768,21 @@ def export_photo(
                             use_photokit=use_photokit,
                             verbose=verbose_,
                         )
+                        results += export_results
+                        for warning_ in export_results.exiftool_warning:
+                            verbose_(f"exiftool warning for file {warning_[0]}: {warning_[1]}")
+                        for error_ in export_results.exiftool_error:
+                            click.echo(click.style(f"exiftool error for file {error_[0]}: {error_[1]}", fg=CLI_COLOR_ERROR),err=True)
 
-                        results_exported.extend(export_results.exported)
-                        results_new.extend(export_results.new)
-                        results_updated.extend(export_results.updated)
-                        results_skipped.extend(export_results.skipped)
-                        results_exif_updated.extend(export_results.exif_updated)
-                        results_touched.extend(export_results.touched)
-                        results_converted.extend(export_results.converted_to_jpeg)
-                        results_sidecar_json_written.extend(
-                            export_results.sidecar_json_written
-                        )
-                        results_sidecar_json_skipped.extend(
-                            export_results.sidecar_json_skipped
-                        )
-                        results_sidecar_xmp_written.extend(
-                            export_results.sidecar_xmp_written
-                        )
-                        results_sidecar_xmp_skipped.extend(
-                            export_results.sidecar_xmp_skipped
-                        )
-
-                    except Exception:
+                    except Exception as e:
                         click.echo(
-                            f"Error exporting photo {photo.original_filename} ({photo.filename}) as {original_filename}",
+                            click.style(
+                                f"Error exporting photo {photo.original_filename} ({photo.filename}) as {original_filename}: {e}",
+                                fg=CLI_COLOR_ERROR,
+                            ),
                             err=True,
                         )
-                        results_error.extend(
+                        results.error.append(
                             str(pathlib.Path(dest) / original_filename)
                         )
             else:
@@ -2871,7 +2832,7 @@ def export_photo(
                 if missing_edited:
                     space = " " if not verbose else ""
                     verbose_(f"{space}Skipping missing edited photo for {filename}")
-                    results_missing.append(
+                    results.missing.append(
                         str(pathlib.Path(dest_path) / edited_filename)
                     )
                 else:
@@ -2902,65 +2863,36 @@ def export_photo(
                             use_photokit=use_photokit,
                             verbose=verbose_,
                         )
-
-                        results_exported.extend(export_results_edited.exported)
-                        results_new.extend(export_results_edited.new)
-                        results_updated.extend(export_results_edited.updated)
-                        results_skipped.extend(export_results_edited.skipped)
-                        results_exif_updated.extend(export_results_edited.exif_updated)
-                        results_touched.extend(export_results_edited.touched)
-                        results_converted.extend(
-                            export_results_edited.converted_to_jpeg
-                        )
-                        results_sidecar_json_written.extend(
-                            export_results_edited.sidecar_json_written
-                        )
-                        results_sidecar_json_skipped.extend(
-                            export_results_edited.sidecar_json_skipped
-                        )
-                        results_sidecar_xmp_written.extend(
-                            export_results_edited.sidecar_xmp_written
-                        )
-                        results_sidecar_xmp_skipped.extend(
-                            export_results_edited.sidecar_xmp_skipped
-                        )
-
-                    except Exception:
+                        results += export_results_edited
+                        for warning_ in export_results_edited.exiftool_warning:
+                            verbose_(f"exiftool warning for file {warning_[0]}: {warning_[1]}")
+                        for error_ in export_results_edited.exiftool_error:
+                            click.echo(click.style(f"exiftool error for file {error_[0]}: {error_[1]}", fg=CLI_COLOR_ERROR),err=True)
+                    except Exception as e:
                         click.echo(
-                            f"Error exporting photo {filename} as {edited_filename}",
+                            click.style(
+                                f"Error exporting photo {filename} as {edited_filename}",
+                                fg=CLI_COLOR_ERROR,
+                            ),
                             err=True,
                         )
-                        results_error.extend(str(pathlib.Path(dest) / edited_filename))
+                        results.error.append(str(pathlib.Path(dest) / edited_filename))
 
             if verbose:
                 if update:
-                    for new in results_new:
+                    for new in results.new:
                         verbose_(f"Exported new file {new}")
-                    for updated in results_updated:
+                    for updated in results.updated:
                         verbose_(f"Exported updated file {updated}")
-                    for skipped in results_skipped:
+                    for skipped in results.skipped:
                         verbose_(f"Skipped up to date file {skipped}")
                 else:
-                    for exported in results_exported:
+                    for exported in results.exported:
                         verbose_(f"Exported {exported}")
-                for touched in results_touched:
+                for touched in results.touched:
                     verbose_(f"Touched date on file {touched}")
 
-    return ExportResults(
-        exported=results_exported,
-        new=results_new,
-        updated=results_updated,
-        skipped=results_skipped,
-        exif_updated=results_exif_updated,
-        touched=results_touched,
-        converted_to_jpeg=results_converted,
-        sidecar_json_written=results_sidecar_json_written,
-        sidecar_json_skipped=results_sidecar_json_skipped,
-        sidecar_xmp_written=results_sidecar_xmp_written,
-        sidecar_xmp_skipped=results_sidecar_xmp_skipped,
-        missing=results_missing,
-        error=results_error,
-    )
+    return results
 
 
 def get_filenames_from_template(photo, filename_template, original_name):
@@ -3111,24 +3043,14 @@ def load_uuid_from_file(filename):
     return uuid
 
 
-def write_export_report(
-    report_file,
-    results_exported,
-    results_new,
-    results_updated,
-    results_skipped,
-    results_exif_updated,
-    results_touched,
-    results_converted,
-    results_sidecar_json_written,
-    results_sidecar_json_skipped,
-    results_sidecar_xmp_written,
-    results_sidecar_xmp_skipped,
-    results_missing,
-    results_error,
-):
+def write_export_report(report_file, results):
 
-    """ write CSV report with results from export """
+    """ write CSV report with results from export
+    
+    Args:
+        report_file: path to report file
+        results: ExportResults object
+    """
 
     # Collect results for reporting
     # TODO: pull this in a separate write_report function
@@ -3146,65 +3068,61 @@ def write_export_report(
             "sidecar_json": 0,
             "missing": 0,
             "error": 0,
+            "exiftool_warning": "",
+            "exiftool_error": "",
         }
-        for result in results_exported
-        + results_new
-        + results_updated
-        + results_skipped
-        + results_exif_updated
-        + results_touched
-        + results_converted
-        + results_sidecar_json_written
-        + results_sidecar_json_skipped
-        + results_sidecar_xmp_written
-        + results_sidecar_xmp_skipped
-        + results_missing
-        + results_error
+        for result in results.all_files()
     }
 
-    for result in results_exported:
+    for result in results.exported:
         all_results[result]["exported"] = 1
 
-    for result in results_new:
+    for result in results.new:
         all_results[result]["new"] = 1
 
-    for result in results_updated:
+    for result in results.updated:
         all_results[result]["updated"] = 1
 
-    for result in results_skipped:
+    for result in results.skipped:
         all_results[result]["skipped"] = 1
 
-    for result in results_exif_updated:
+    for result in results.exif_updated:
         all_results[result]["exif_updated"] = 1
 
-    for result in results_touched:
+    for result in results.touched:
         all_results[result]["touched"] = 1
 
-    for result in results_converted:
+    for result in results.converted_to_jpeg:
         all_results[result]["converted_to_jpeg"] = 1
 
-    for result in results_sidecar_xmp_written:
+    for result in results.sidecar_xmp_written:
         all_results[result]["sidecar_xmp"] = 1
         all_results[result]["exported"] = 1
 
-    for result in results_sidecar_xmp_skipped:
+    for result in results.sidecar_xmp_skipped:
         all_results[result]["sidecar_xmp"] = 1
         all_results[result]["skipped"] = 1
 
-    for result in results_sidecar_json_written:
+    for result in results.sidecar_json_written:
         all_results[result]["sidecar_json"] = 1
         all_results[result]["exported"] = 1
 
-    for result in results_sidecar_json_skipped:
+    for result in results.sidecar_json_skipped:
         all_results[result]["sidecar_json"] = 1
         all_results[result]["skipped"] = 1
 
-    for result in results_missing:
+    for result in results.missing:
         all_results[result]["missing"] = 1
 
-    for result in results_error:
+    for result in results.error:
         all_results[result]["error"] = 1
 
+    for result in results.exiftool_warning:
+        all_results[result[0]]["exiftool_warning"] = result[1]
+
+    for result in results.exiftool_error:
+        all_results[result[0]]["exiftool_error"] = result[1]
+
     report_columns = [
         "filename",
         "exported",
@@ -3218,6 +3136,8 @@ def write_export_report(
         "sidecar_json",
         "missing",
         "error",
+        "exiftool_warning",
+        "exiftool_error",
     ]
 
     try:
@@ -3227,7 +3147,10 @@ def write_export_report(
             for data in [result for result in all_results.values()]:
                 writer.writerow(data)
     except IOError:
-        click.echo("Could not open output file for writing", err=True)
+        click.echo(
+            click.style("Could not open output file for writing", fg=CLI_COLOR_ERROR),
+            err=True,
+        )
         raise click.Abort()
 
 

osxphotos/_constants.py

@@ -118,4 +118,6 @@ DEFAULT_EDITED_SUFFIX = "_edited"
 # Default suffix to add to original images
 DEFAULT_ORIGINAL_SUFFIX = ""
 
-
+# Colors for print CLI messages
+CLI_COLOR_ERROR = 'red'
+CLI_COLOR_WARNING = 'yellow'

osxphotos/_version.py

@@ -1,5 +1,5 @@
 """ version info """
 
-__version__ = "0.38.5"
+__version__ = "0.38.8"
 
 

osxphotos/exiftool.py

@@ -145,6 +145,7 @@ class ExifTool:
         self.file = filepath
         self.overwrite = overwrite
         self.data = {}
+        self.warning = None
         self.error = None
         # if running as a context manager, self._context_mgr will be True
         self._context_mgr = False
@@ -163,6 +164,7 @@ class ExifTool:
             True if success otherwise False
             
             If error generated by exiftool, returns False and sets self.error to error string
+            If warning generated by exiftool, returns True (unless there was also an error) and sets self.warning to warning string
             If called in context manager, returns True (execution is delayed until exiting context manager)
         """
 
@@ -175,8 +177,8 @@ class ExifTool:
             self._commands.extend(command)
             return True
         else:
-            _, self.error = self.run_commands(*command)
-            return self.error is None
+            _, _, error = self.run_commands(*command)
+            return error is None
 
     def addvalues(self, tag, *values):
         """ Add one or more value(s) to tag
@@ -190,6 +192,7 @@ class ExifTool:
             True if success otherwise False
             
             If error generated by exiftool, returns False and sets self.error to error string
+            If warning generated by exiftool, returns True (unless there was also an error) and sets self.warning to warning string
             If called in context manager, returns True (execution is delayed until exiting context manager)
             
         Notes: exiftool may add duplicate values for some tags so the caller must ensure
@@ -216,8 +219,8 @@ class ExifTool:
             self._commands.extend(command)
             return True
         else:
-            _, self.error = self.run_commands(*command)
-            return self.error is None
+            _, _, error = self.run_commands(*command)
+            return error is None
 
     def run_commands(self, *commands, no_file=False):
         """ Run commands in the exiftool process and return result.
@@ -228,11 +231,12 @@ class ExifTool:
                         by default, all commands will be run against self.file
                         use no_file=True to run a command without passing the filename
         Returns:
-            (output, errror)
+            (output, warning, errror)
             output: bytes is containing output of exiftool commands
-            error: if exiftool generated an error, bytes containing error string otherwise None
+            warning: if exiftool generated warnings, string containing warning otherwise empty string
+            error: if exiftool generated errors, string containing otherwise empty string
 
-        Note: Also sets self.error if error generated.
+        Note: Also sets self.warning and self.error if warning or error generated.
         """
         if not (hasattr(self, "_process") and self._process):
             raise ValueError("exiftool process is not running")
@@ -259,16 +263,21 @@ class ExifTool:
 
         # read the output
         output = b""
+        warning = b""
         error = b""
         while EXIFTOOL_STAYOPEN_EOF not in str(output):
             line = self._process.stdout.readline()
             if line.startswith(b"Warning"):
-                error += line
+                warning += line.strip()
+            elif line.startswith(b"Error"):
+                error += line.strip()
             else:
                 output += line.strip()
-        error = None if error == b"" else error
+        warning = "" if warning == b"" else warning.decode("utf-8")
+        error = "" if error == b"" else error.decode("utf-8")
+        self.warning = warning
         self.error = error
-        return output[:-EXIFTOOL_STAYOPEN_EOF_LEN], error
+        return output[:-EXIFTOOL_STAYOPEN_EOF_LEN], warning, error
 
     @property
     def pid(self):
@@ -278,14 +287,14 @@ class ExifTool:
     @property
     def version(self):
         """ returns exiftool version """
-        ver, _ = self.run_commands("-ver", no_file=True)
+        ver, _, _ = self.run_commands("-ver", no_file=True)
         return ver.decode("utf-8")
 
     def asdict(self):
         """ return dictionary of all EXIF tags and values from exiftool 
             returns empty dict if no tags
         """
-        json_str, _ = self.run_commands("-json")
+        json_str, _, _ = self.run_commands("-json")
         if json_str:
             exifdict = json.loads(json_str)
             return exifdict[0]
@@ -294,7 +303,7 @@ class ExifTool:
 
     def json(self):
         """ returns JSON string containing all EXIF tags and values from exiftool """
-        json, _ = self.run_commands("-json")
+        json, _, _ = self.run_commands("-json")
         return json
 
     def _read_exif(self):
@@ -314,4 +323,5 @@ class ExifTool:
         if exc_type:
             return False
         elif self._commands:
-            _, self.error = self.run_commands(*self._commands)
+            # run_commands sets self.warning and self.error as needed
+            self.run_commands(*self._commands)

osxphotos/photoinfo/_photoinfo_export.py

@@ -45,24 +45,105 @@ from ..photokit import (
 )
 from ..utils import dd_to_dms_str, findfiles, noop
 
-ExportResults = namedtuple(
-    "ExportResults",
-    [
-        "exported",
-        "new",
-        "updated",
-        "skipped",
-        "exif_updated",
-        "touched",
-        "converted_to_jpeg",
-        "sidecar_json_written",
-        "sidecar_json_skipped",
-        "sidecar_xmp_written",
-        "sidecar_xmp_skipped",
-        "missing",
-        "error",
-    ],
-)
+
+class ExportResults:
+    """ holds export results for export2 """
+
+    def __init__(
+        self,
+        exported=None,
+        new=None,
+        updated=None,
+        skipped=None,
+        exif_updated=None,
+        touched=None,
+        converted_to_jpeg=None,
+        sidecar_json_written=None,
+        sidecar_json_skipped=None,
+        sidecar_xmp_written=None,
+        sidecar_xmp_skipped=None,
+        missing=None,
+        error=None,
+        exiftool_warning=None,
+        exiftool_error=None,
+    ):
+        self.exported = exported or []
+        self.new = new or []
+        self.updated = updated or []
+        self.skipped = skipped or []
+        self.exif_updated = exif_updated or []
+        self.touched = touched or []
+        self.converted_to_jpeg = converted_to_jpeg or []
+        self.sidecar_json_written = sidecar_json_written or []
+        self.sidecar_json_skipped = sidecar_json_skipped or []
+        self.sidecar_xmp_written = sidecar_xmp_written or []
+        self.sidecar_xmp_skipped = sidecar_xmp_skipped or []
+        self.missing = missing or []
+        self.error = error or []
+        self.exiftool_warning = exiftool_warning or []
+        self.exiftool_error = exiftool_error or []
+
+    def all_files(self):
+        """ return all filenames contained in results """
+        files = (
+            self.exported
+            + self.new
+            + self.updated
+            + self.skipped
+            + self.exif_updated
+            + self.touched
+            + self.converted_to_jpeg
+            + self.sidecar_json_written
+            + self.sidecar_json_skipped
+            + self.sidecar_xmp_written
+            + self.sidecar_xmp_skipped
+            + self.missing
+            + self.error
+        )
+        files += [x[0] for x in self.exiftool_warning]
+        files += [x[0] for x in self.exiftool_error]
+
+        files = list(set(files))
+        return files
+
+    def __iadd__(self, other):
+        self.exported += other.exported
+        self.new += other.new
+        self.updated += other.updated
+        self.skipped += other.skipped
+        self.exif_updated += other.exif_updated
+        self.touched += other.touched
+        self.converted_to_jpeg += other.converted_to_jpeg
+        self.sidecar_json_written += other.sidecar_json_written
+        self.sidecar_json_skipped += other.sidecar_json_skipped
+        self.sidecar_xmp_written += other.sidecar_xmp_written
+        self.sidecar_xmp_skipped += other.sidecar_xmp_skipped
+        self.missing += other.missing
+        self.error += other.error
+        self.exiftool_warning += other.exiftool_warning
+        self.exiftool_error += other.exiftool_error
+        return self
+
+    def __str__(self):
+        return (
+            "ExportResults("
+            + f"exported={self.exported}"
+            + f",new={self.new}"
+            + f",updated={self.updated}"
+            + f",skipped={self.skipped}"
+            + f",exif_updated={self.exif_updated}"
+            + f",touched={self.touched}"
+            + f",converted_to_jpeg={self.converted_to_jpeg}"
+            + f",sidecar_json_written={self.sidecar_json_written}"
+            + f",sidecar_json_skipped={self.sidecar_json_skipped}"
+            + f",sidecar_xmp_written={self.sidecar_xmp_written}"
+            + f",sidecar_xmp_skipped={self.sidecar_xmp_skipped}"
+            + f",missing={self.missing}"
+            + f",error={self.error}"
+            + f",exiftool_warning={self.exiftool_warning}"
+            + f",exiftool_error={self.exiftool_error}"
+            + ")"
+        )
 
 
 # hexdigest is not a class method, don't import this into PhotoInfo
@@ -389,7 +470,8 @@ def export2(
     ignore_date_modified: for use with sidecar and exiftool; if True, sets EXIF:ModifyDate to EXIF:DateTimeOriginal even if date_modified is set
     verbose: optional callable function to use for printing verbose text during processing; if None (default), does not print output.
 
-    Returns: ExportResults namedtuple with fields: 
+    Returns: ExportResults class 
+        ExportResults has attributes: 
         "exported",
         "new",
         "updated",
@@ -402,7 +484,10 @@ def export2(
         "sidecar_xmp_written",
         "sidecar_xmp_skipped",
         "missing",
-        "error"
+        "error",
+        "exiftool_warning",
+        "exiftool_error",
+
  
     Note: to use dry run mode, you must set dry_run=True and also pass in memory version of export_db,
           and no-op fileutil (e.g. ExportDBInMemory and FileUtilNoOp)
@@ -853,6 +938,10 @@ def export2(
         exif_files = exported_files
 
     exif_files_updated = []
+    exiftool_warning = []
+    exiftool_error = []
+    errors = []
+    # TODO: remove duplicative code from below
     if exiftool and update and exif_files:
         for exported_file in exif_files:
             files_are_different = False
@@ -876,7 +965,7 @@ def export2(
                 # or files were different
                 verbose(f"Writing metadata with exiftool for {exported_file}")
                 if not dry_run:
-                    self._write_exif_data(
+                    warning_, error_ = self._write_exif_data(
                         exported_file,
                         use_albums_as_keywords=use_albums_as_keywords,
                         use_persons_as_keywords=use_persons_as_keywords,
@@ -884,6 +973,12 @@ def export2(
                         description_template=description_template,
                         ignore_date_modified=ignore_date_modified,
                     )
+                    if warning_:
+                        exiftool_warning.append((exported_file, warning_))
+                    if error_:
+                        exiftool_error.append((exported_file, error_))
+                        errors.append(exported_file)
+
                 export_db.set_exifdata_for_file(
                     exported_file,
                     self._exiftool_json_sidecar(
@@ -904,7 +999,7 @@ def export2(
         for exported_file in exif_files:
             verbose(f"Writing metadata with exiftool for {exported_file}")
             if not dry_run:
-                self._write_exif_data(
+                warning_, error_ = self._write_exif_data(
                     exported_file,
                     use_albums_as_keywords=use_albums_as_keywords,
                     use_persons_as_keywords=use_persons_as_keywords,
@@ -912,6 +1007,11 @@ def export2(
                     description_template=description_template,
                     ignore_date_modified=ignore_date_modified,
                 )
+                if warning_:
+                    exiftool_warning.append((exported_file, warning_))
+                if error_:
+                    exiftool_error.append((exported_file, error_))
+                    errors.append(exported_file)
 
             export_db.set_exifdata_for_file(
                 exported_file,
@@ -949,8 +1049,9 @@ def export2(
         sidecar_json_skipped=sidecar_json_files_skipped,
         sidecar_xmp_written=sidecar_xmp_files_written,
         sidecar_xmp_skipped=sidecar_xmp_files_skipped,
-        missing=[],
-        error=[],
+        error=errors,
+        exiftool_error=exiftool_error,
+        exiftool_warning=exiftool_warning,
     )
     return results
 
@@ -1152,6 +1253,9 @@ def _write_exif_data(
         use_persons_as_keywords: treat person names as keywords
         keyword_template: (list of strings); list of template strings to render as keywords
         ignore_date_modified: if True, sets EXIF:ModifyDate to EXIF:DateTimeOriginal even if date_modified is set
+
+    Returns:
+        (warning, error) of warning and error strings if exiftool produces warnings or errors
     """
     if not os.path.exists(filepath):
         raise FileNotFoundError(f"Could not find file {filepath}")
@@ -1170,6 +1274,7 @@ def _write_exif_data(
                     exiftool.setvalue(exiftag, v)
             else:
                 exiftool.setvalue(exiftag, val)
+    return exiftool.warning, exiftool.error
 
 
 def _exiftool_dict(
@@ -1236,13 +1341,13 @@ def _exiftool_dict(
     person_list = []
     if self.persons:
         # filter out _UNKNOWN_PERSON
-        person_list = sorted([p for p in self.persons if p != _UNKNOWN_PERSON])
+        person_list = [p for p in self.persons if p != _UNKNOWN_PERSON]
 
     if use_persons_as_keywords and person_list:
-        keyword_list.extend(sorted(person_list))
+        keyword_list.extend(person_list)
 
     if use_albums_as_keywords and self.albums:
-        keyword_list.extend(sorted(self.albums))
+        keyword_list.extend(self.albums)
 
     if keyword_template:
         rendered_keywords = []
@@ -1277,16 +1382,19 @@ def _exiftool_dict(
         keyword_list.extend(rendered_keywords)
 
     if keyword_list:
+        # remove duplicates
+        keyword_list = sorted(list(set(keyword_list)))
         exif["XMP:TagsList"] = keyword_list.copy()
         exif["IPTC:Keywords"] = keyword_list.copy()
 
     if person_list:
+        person_list = sorted(list(set(person_list)))
         exif["XMP:PersonInImage"] = person_list.copy()
 
     if self.keywords or person_list:
         # Photos puts both keywords and persons in Subject when using "Export IPTC as XMP"
         # only use Photos' keywords for subject (e.g. don't include template values)
-        exif["XMP:Subject"] = self.keywords.copy() + person_list.copy()
+        exif["XMP:Subject"] = sorted(list(set(self.keywords + person_list)))
 
     # if self.favorite():
     #     exif["Rating"] = 5
@@ -1355,13 +1463,12 @@ def _exiftool_dict(
         date_utc = datetime_tz_to_utc(date)
         creationdate = date_utc.strftime("%Y:%m:%d %H:%M:%S")
         exif["QuickTime:CreateDate"] = creationdate
-        if self.date_modified is not None and not ignore_date_modified:
+        if self.date_modified is None or ignore_date_modified:
+            exif["QuickTime:ModifyDate"] = creationdate
+        else:
             exif["QuickTime:ModifyDate"] = datetime_tz_to_utc(
                 self.date_modified
             ).strftime("%Y:%m:%d %H:%M:%S")
-        else:
-            exif["QuickTime:ModifyDate"] = creationdate
-
     return exif
 
 
@@ -1499,6 +1606,15 @@ def _xmp_sidecar(
         # Photos puts both keywords and persons in Subject when using "Export IPTC as XMP"
         subject_list = list(self.keywords) + person_list
 
+    # remove duplicates
+    # sorted mainly to make testing the XMP file easier
+    if keyword_list:
+        keyword_list = sorted(list(set(keyword_list)))
+    if subject_list:
+        subject_list = sorted(list(set(subject_list)))
+    if person_list:
+        person_list = sorted(list(set(person_list)))
+
     xmp_str = xmp_template.render(
         photo=self,
         description=description,

osxphotos/phototemplate.py

@@ -510,7 +510,7 @@ class PhotoTemplate:
                     subfield = field[9:]
 
                     if not self.photo.path:
-                        values = []
+                        values = [None]
                     else:
                         exif = ExifTool(self.photo.path)
                         exifdict = exif.asdict()
@@ -870,7 +870,7 @@ class PhotoTemplate:
         """
 
         """ return list of values for a multi-valued template field """
-        values = [] 
+        values = []
         if field == "album":
             values = self.photo.albums
         elif field == "keyword":

tests/Test-10.15.7.photoslibrary/database/Photos.sqlite.lock

@@ -7,7 +7,7 @@
 	<key>hostuuid</key>
 	<string>9575E48B-8D5F-5654-ABAC-4431B1167324</string>
 	<key>pid</key>
-	<integer>485</integer>
+	<integer>55247</integer>
 	<key>processname</key>
 	<string>photolibraryd</string>
 	<key>uid</key>

tests/test_catalina_10_15_7.py

@@ -18,10 +18,10 @@ PHOTOS_DB = "tests/Test-10.15.7.photoslibrary/database/photos.db"
 PHOTOS_DB_PATH = "/Test-10.15.7.photoslibrary/database/photos.db"
 PHOTOS_LIBRARY_PATH = "/Test-10.15.7.photoslibrary"
 
-PHOTOS_DB_LEN = 18
-PHOTOS_NOT_IN_TRASH_LEN = 16
+PHOTOS_DB_LEN = 19
+PHOTOS_NOT_IN_TRASH_LEN = 17
 PHOTOS_IN_TRASH_LEN = 2
-PHOTOS_DB_IMPORT_SESSIONS = 13
+PHOTOS_DB_IMPORT_SESSIONS = 14
 
 KEYWORDS = [
     "Kids",
@@ -35,6 +35,7 @@ KEYWORDS = [
     "United Kingdom",
     "foo/bar",
     "Travel",
+    "Maria",
 ]
 # Photos 5 includes blank person for detected face
 PERSONS = ["Katie", "Suzy", "Maria", _UNKNOWN_PERSON]
@@ -60,6 +61,7 @@ KEYWORDS_DICT = {
     "United Kingdom": 1,
     "foo/bar": 1,
     "Travel": 2,
+    "Maria": 1,
 }
 PERSONS_DICT = {"Katie": 3, "Suzy": 2, "Maria": 2, _UNKNOWN_PERSON: 1}
 ALBUM_DICT = {
@@ -339,7 +341,7 @@ def test_attributes_2(photosdb):
     photos = photosdb.photos(uuid=[UUID_DICT["has_adjustments"]])
     assert len(photos) == 1
     p = photos[0]
-    assert p.keywords == ["wedding"]
+    assert sorted(p.keywords) == ["Maria", "wedding"]
     assert p.original_filename == "wedding.jpg"
     assert p.filename == "E9BC5C36-7CD1-40A1-A72B-8B8FAC227D51.jpeg"
     assert p.date == datetime.datetime(
@@ -971,7 +973,7 @@ def test_from_to_date(photosdb):
     time.tzset()
 
     photos = photosdb.photos(from_date=datetime.datetime(2018, 10, 28))
-    assert len(photos) == 9
+    assert len(photos) == 10 
 
     photos = photosdb.photos(to_date=datetime.datetime(2018, 10, 28))
     assert len(photos) == 7

tests/test_cli.py

@@ -405,6 +405,12 @@ CLI_EXIFTOOL_IGNORE_DATE_MODIFIED = {
     }
 }
 
+CLI_EXIFTOOL_ERROR = ["E2078879-A29C-4D6F-BACB-E3BBE6C3EB91"]
+
+CLI_EXIFTOOL_DUPLICATE_KEYWORDS = {
+    "E9BC5C36-7CD1-40A1-A72B-8B8FAC227D51": "wedding.jpg"
+}
+
 LABELS_JSON = {
     "labels": {
         "Plant": 7,
@@ -1015,7 +1021,10 @@ def test_export_exiftool():
 
             exif = ExifTool(CLI_EXIFTOOL[uuid]["File:FileName"]).asdict()
             for key in CLI_EXIFTOOL[uuid]:
-                assert exif[key] == CLI_EXIFTOOL[uuid][key]
+                if type(exif[key]) == list:
+                    assert sorted(exif[key]) == sorted(CLI_EXIFTOOL[uuid][key])
+                else:
+                    assert exif[key] == CLI_EXIFTOOL[uuid][key]
 
 
 @pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
@@ -1049,7 +1058,10 @@ def test_export_exiftool_ignore_date_modified():
                 CLI_EXIFTOOL_IGNORE_DATE_MODIFIED[uuid]["File:FileName"]
             ).asdict()
             for key in CLI_EXIFTOOL_IGNORE_DATE_MODIFIED[uuid]:
-                assert exif[key] == CLI_EXIFTOOL_IGNORE_DATE_MODIFIED[uuid][key]
+                if type(exif[key]) == list:
+                    assert sorted(exif[key]) == sorted(CLI_EXIFTOOL_IGNORE_DATE_MODIFIED[uuid][key])
+                else:
+                    assert exif[key] == CLI_EXIFTOOL_IGNORE_DATE_MODIFIED[uuid][key]
 
 
 @pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
@@ -1092,6 +1104,74 @@ def test_export_exiftool_quicktime():
                 os.unlink(filename)
 
 
+@pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
+def test_export_exiftool_duplicate_keywords():
+    """ ensure duplicate keywords are removed """
+    import glob
+    import os
+    import os.path
+    from osxphotos.__main__ import export
+    from osxphotos.exiftool import ExifTool
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        for uuid in CLI_EXIFTOOL_DUPLICATE_KEYWORDS:
+            result = runner.invoke(
+                export,
+                [
+                    os.path.join(cwd, PHOTOS_DB_15_7),
+                    ".",
+                    "-V",
+                    "--exiftool",
+                    "--uuid",
+                    f"{uuid}",
+                ],
+            )
+            exif = ExifTool(CLI_EXIFTOOL_DUPLICATE_KEYWORDS[uuid])
+            exifdict = exif.asdict()
+            assert sorted(exifdict["IPTC:Keywords"]) == ["Maria", "wedding"]
+            assert sorted(exifdict["XMP:Subject"]) == ["Maria", "wedding"]
+
+
+@pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
+def test_export_exiftool_error():
+    """" test --exiftool catching error """
+    import glob
+    import os
+    import os.path
+    from osxphotos.__main__ import export
+    from osxphotos.exiftool import ExifTool
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        for uuid in CLI_EXIFTOOL:
+            result = runner.invoke(
+                export,
+                [
+                    os.path.join(cwd, PHOTOS_DB_15_7),
+                    ".",
+                    "-V",
+                    "--exiftool",
+                    "--uuid",
+                    f"{uuid}",
+                ],
+            )
+            assert result.exit_code == 0
+            files = glob.glob("*")
+            assert sorted(files) == sorted([CLI_EXIFTOOL[uuid]["File:FileName"]])
+
+            exif = ExifTool(CLI_EXIFTOOL[uuid]["File:FileName"]).asdict()
+            for key in CLI_EXIFTOOL[uuid]:
+                if type(exif[key]) == list:
+                    assert sorted(exif[key]) == sorted(CLI_EXIFTOOL[uuid][key])
+                else:
+                    assert exif[key] == CLI_EXIFTOOL[uuid][key]
+
+
 def test_export_edited_suffix():
     """ test export with --edited-suffix """
     import glob
@@ -3797,6 +3877,7 @@ def test_export_touch_files_exiftool_update():
 
 def test_export_ignore_signature():
     """ test export with --ignore-signature """
+    import os
     from osxphotos.__main__ import export
 
     runner = CliRunner()
@@ -3843,6 +3924,135 @@ def test_export_ignore_signature():
         assert "exported: 0, updated: 0" in result.output
 
 
+def test_export_ignore_signature_sidecar():
+    """ test export with --ignore-signature and --sidecar """
+    """
+    Test the following use cases: 
+    If the metadata (in Photos) that went into the sidecar did not change, the sidecar will not be updated
+    If the metadata (in Photos) that went into the sidecar did change, a new sidecar is written but a new image file is not
+    If a sidecar does not exist for the photo, a sidecar will be written whether or not the photo file was written
+    """
+
+    import osxphotos
+    import os
+
+    from osxphotos.__main__ import export
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        # first, export some files
+        result = runner.invoke(
+            export, [os.path.join(cwd, PHOTOS_DB_15_7), ".", "-V", "--sidecar", "XMP"]
+        )
+        assert result.exit_code == 0
+
+        # export with --update and --ignore-signature
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_7),
+                ".",
+                "-V",
+                "--update",
+                "--sidecar",
+                "XMP",
+                "--ignore-signature",
+            ],
+        )
+        assert result.exit_code == 0
+        assert "exported: 0, updated: 0" in result.output
+        assert "Writing XMP sidecar" not in result.output
+
+        # modify a couple of files
+        for filename in CLI_EXPORT_IGNORE_SIGNATURE_FILENAMES:
+            modify_file(f"./{filename}")
+
+        # export with --update and --ignore-signature
+        # which should ignore the two modified files
+        # sidecar files should not be re-written
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_7),
+                ".",
+                "-V",
+                "--update",
+                "--sidecar",
+                "XMP",
+                "--ignore-signature",
+            ],
+        )
+        assert result.exit_code == 0
+        assert "exported: 0" in result.output
+        assert "Writing XMP sidecar" not in result.output
+
+        # change the sidecar data in export DB
+        # should result in a new sidecar being exported but not the image itself
+        exportdb = osxphotos.export_db.ExportDB("./.osxphotos_export.db")
+        for filename in CLI_EXPORT_IGNORE_SIGNATURE_FILENAMES:
+            exportdb.set_sidecar_for_file(f"{filename}.xmp", "FOO", (0, 1, 2))
+
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_7),
+                ".",
+                "-V",
+                "--update",
+                "--ignore-signature",
+                "--sidecar",
+                "XMP",
+            ],
+        )
+        assert result.exit_code == 0
+        assert "exported: 0, updated: 0" in result.output
+        assert result.output.count("Writing XMP sidecar") == len(
+            CLI_EXPORT_IGNORE_SIGNATURE_FILENAMES
+        )
+
+        # run --update again, should be 0 files exported
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_7),
+                ".",
+                "-V",
+                "--update",
+                "--ignore-signature",
+                "--sidecar",
+                "XMP",
+            ],
+        )
+        assert result.exit_code == 0
+        assert "exported: 0, updated: 0" in result.output
+        assert "Writing XMP sidecar" not in result.output
+
+        # remove XMP files and run again to verify the files get written
+        for filename in CLI_EXPORT_IGNORE_SIGNATURE_FILENAMES:
+            os.unlink(f"./{filename}.xmp")
+
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_7),
+                ".",
+                "-V",
+                "--update",
+                "--ignore-signature",
+                "--sidecar",
+                "XMP",
+            ],
+        )
+        assert result.exit_code == 0
+        assert "exported: 0, updated: 0" in result.output
+        assert result.output.count("Writing XMP sidecar") == len(
+            CLI_EXPORT_IGNORE_SIGNATURE_FILENAMES
+        )
+
+
 def test_labels():
     """Test osxphotos labels """
     import json

tests/test_exiftool.py

@@ -2,6 +2,7 @@ import pytest
 from osxphotos.exiftool import get_exiftool_path
 
 TEST_FILE_ONE_KEYWORD = "tests/test-images/wedding.jpg"
+TEST_FILE_BAD_IMAGE = "tests/test-images/badimage.jpeg"
 TEST_FILE_MULTI_KEYWORD = "tests/test-images/Tulips.jpg"
 TEST_MULTI_KEYWORDS = [
     "Top Shot",
@@ -109,8 +110,8 @@ def test_setvalue_1():
     assert exif.data["IPTC:Keywords"] == "test"
 
 
-def test_setvalue_error():
-    # test setting illegal tag value generates error
+def test_setvalue_warning():
+    # test setting illegal tag value generates warning
     import os.path
     import tempfile
     import osxphotos.exiftool
@@ -122,6 +123,22 @@ def test_setvalue_error():
 
     exif = osxphotos.exiftool.ExifTool(tempfile)
     exif.setvalue("IPTC:Foo", "test")
+    assert exif.warning
+
+
+def test_setvalue_error():
+    # test setting tag on bad image generates error
+    import os.path
+    import tempfile
+    import osxphotos.exiftool
+    from osxphotos.fileutil import FileUtil
+
+    tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    tempfile = os.path.join(tempdir.name, os.path.basename(TEST_FILE_BAD_IMAGE))
+    FileUtil.copy(TEST_FILE_BAD_IMAGE, tempfile)
+
+    exif = osxphotos.exiftool.ExifTool(tempfile)
+    exif.setvalue("IPTC:Keywords", "test")
     assert exif.error
 
 
@@ -142,7 +159,7 @@ def test_setvalue_context_manager():
         exif.setvalue("XMP:Title", "title")
         exif.setvalue("XMP:Subject", "subject")
 
-    assert exif.error is None
+    assert not exif.error
 
     exif2 = osxphotos.exiftool.ExifTool(tempfile)
     exif2._read_exif()
@@ -151,8 +168,8 @@ def test_setvalue_context_manager():
     assert exif2.data["XMP:Subject"] == "subject"
 
 
-def test_setvalue_context_manager_error():
-    # test setting a tag value as context manager when error generated
+def test_setvalue_context_manager_warning():
+    # test setting a tag value as context manager when warning generated
     import os.path
     import tempfile
     import osxphotos.exiftool
@@ -164,6 +181,22 @@ def test_setvalue_context_manager_error():
 
     with osxphotos.exiftool.ExifTool(tempfile) as exif:
         exif.setvalue("Foo:Bar", "test1")
+    assert exif.warning
+
+
+def test_setvalue_context_manager_error():
+    # test setting a tag value as context manager when error generated
+    import os.path
+    import tempfile
+    import osxphotos.exiftool
+    from osxphotos.fileutil import FileUtil
+
+    tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    tempfile = os.path.join(tempdir.name, os.path.basename(TEST_FILE_BAD_IMAGE))
+    FileUtil.copy(TEST_FILE_BAD_IMAGE, tempfile)
+
+    with osxphotos.exiftool.ExifTool(tempfile) as exif:
+        exif.setvalue("IPTC:Keywords", "test1")
     assert exif.error
 
 

tests/test_export_catalina_10_15_7.py

@@ -22,6 +22,7 @@ KEYWORDS = [
     "St. James's Park",
     "UK",
     "United Kingdom",
+    "Maria"
 ]
 # Photos 5 includes blank person for detected face
 PERSONS = ["Katie", "Suzy", "Maria", _UNKNOWN_PERSON]
@@ -39,6 +40,7 @@ KEYWORDS_DICT = {
     "St. James's Park": 1,
     "UK": 1,
     "United Kingdom": 1,
+    "Maria": 1,
 }
 PERSONS_DICT = {"Katie": 3, "Suzy": 2, "Maria": 1, _UNKNOWN_PERSON: 1}
 ALBUM_DICT = {
@@ -70,8 +72,8 @@ EXIF_JSON_UUID = UUID_DICT["has_adjustments"]
 EXIF_JSON_EXPECTED = """ 
     [{"EXIF:ImageDescription": "Bride Wedding day", 
     "XMP:Description": "Bride Wedding day", 
-    "XMP:TagsList": ["wedding"], 
-    "IPTC:Keywords": ["wedding"], 
+    "XMP:TagsList": ["Maria", "wedding"], 
+    "IPTC:Keywords": ["Maria", "wedding"], 
     "XMP:PersonInImage": ["Maria"], 
     "XMP:Subject": ["wedding", "Maria"], 
     "EXIF:DateTimeOriginal": "2019:04:15 14:40:24", 
@@ -85,8 +87,8 @@ EXIF_JSON_EXPECTED = """
 EXIF_JSON_EXPECTED_IGNORE_DATE_MODIFIED = """ 
     [{"EXIF:ImageDescription": "Bride Wedding day", 
     "XMP:Description": "Bride Wedding day", 
-    "XMP:TagsList": ["wedding"], 
-    "IPTC:Keywords": ["wedding"], 
+    "XMP:TagsList": ["Maria", "wedding"], 
+    "IPTC:Keywords": ["Maria", "wedding"], 
     "XMP:PersonInImage": ["Maria"], 
     "XMP:Subject": ["wedding", "Maria"], 
     "EXIF:DateTimeOriginal": "2019:04:15 14:40:24", 
@@ -522,8 +524,8 @@ def test_exiftool_json_sidecar_keyword_template_long(caplog):
         """
         [{"EXIF:ImageDescription": "Bride Wedding day", 
         "XMP:Description": "Bride Wedding day", 
-        "XMP:TagsList": ["wedding", "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"], 
-        "IPTC:Keywords": ["wedding", "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"], 
+        "XMP:TagsList": ["Maria", "wedding", "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"], 
+        "IPTC:Keywords": ["Maria", "wedding", "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"], 
         "XMP:PersonInImage": ["Maria"], 
         "XMP:Subject": ["wedding", "Maria"], 
         "EXIF:DateTimeOriginal": "2019:04:15 14:40:24", 
@@ -571,8 +573,8 @@ def test_exiftool_json_sidecar_keyword_template():
         """
         [{"EXIF:ImageDescription": "Bride Wedding day", 
         "XMP:Description": "Bride Wedding day", 
-        "XMP:TagsList": ["wedding", "Folder1/SubFolder2/AlbumInFolder", "I have a deleted twin"], 
-        "IPTC:Keywords": ["wedding", "Folder1/SubFolder2/AlbumInFolder", "I have a deleted twin"], 
+        "XMP:TagsList": ["Maria", "wedding", "Folder1/SubFolder2/AlbumInFolder", "I have a deleted twin"], 
+        "IPTC:Keywords": ["Maria", "wedding", "Folder1/SubFolder2/AlbumInFolder", "I have a deleted twin"], 
         "XMP:PersonInImage": ["Maria"], 
         "XMP:Subject": ["wedding", "Maria"],
         "EXIF:DateTimeOriginal": "2019:04:15 14:40:24", 
@@ -716,7 +718,7 @@ def test_xmp_sidecar_is_valid(tmp_path):
     xmp_file = tmp_path / XMP_FILENAME
     assert xmp_file.is_file()
     exiftool = ExifTool(str(xmp_file))
-    output, _ = exiftool.run_commands("-validate", "-warning")
+    output, _, _ = exiftool.run_commands("-validate", "-warning")
     assert output == b"[ExifTool]      Validate                        : 0 0 0"
 
 

tests/test_export_mojave_10_14_6.py

@@ -427,9 +427,9 @@ def test_xmp_sidecar():
         <!-- keywords and persons listed in <dc:subject> as Photos does -->
         <dc:subject>
             <rdf:Seq>
+                <rdf:li>Katie</rdf:li>
                 <rdf:li>Kids</rdf:li>
                 <rdf:li>Suzy</rdf:li>
-                <rdf:li>Katie</rdf:li>
             </rdf:Seq>
         </dc:subject>
         <photoshop:DateCreated>2018-09-28T15:35:49.063000-04:00</photoshop:DateCreated>
@@ -438,8 +438,8 @@ def test_xmp_sidecar():
             xmlns:Iptc4xmpExt='http://iptc.org/std/Iptc4xmpExt/2008-02-29/'>
         <Iptc4xmpExt:PersonInImage>
             <rdf:Bag>
-                    <rdf:li>Suzy</rdf:li>
                     <rdf:li>Katie</rdf:li>
+                    <rdf:li>Suzy</rdf:li>
             </rdf:Bag>
         </Iptc4xmpExt:PersonInImage>
         </rdf:Description>

tests/test_exportresults.py

@@ -0,0 +1,95 @@
+""" test ExportResults class """
+
+import pytest
+from osxphotos.photoinfo import ExportResults
+
+EXPORT_RESULT_ATTRIBUTES = [
+    "exported",
+    "new",
+    "updated",
+    "skipped",
+    "exif_updated",
+    "touched",
+    "converted_to_jpeg",
+    "sidecar_json_written",
+    "sidecar_json_skipped",
+    "sidecar_xmp_written",
+    "sidecar_xmp_skipped",
+    "missing",
+    "error",
+    "exiftool_warning",
+    "exiftool_error",
+]
+
+
+def test_exportresults_init():
+    results = ExportResults()
+    assert results.exported == []
+    assert results.new == []
+    assert results.updated == []
+    assert results.skipped == []
+    assert results.exif_updated == []
+    assert results.touched == []
+    assert results.converted_to_jpeg == []
+    assert results.sidecar_json_written == []
+    assert results.sidecar_json_skipped == []
+    assert results.sidecar_xmp_written == []
+    assert results.sidecar_xmp_skipped == []
+    assert results.missing == []
+    assert results.error == []
+    assert results.exiftool_warning == []
+    assert results.exiftool_error == []
+
+
+def test_exportresults_iadd():
+    results1 = ExportResults()
+    results2 = ExportResults()
+    for x in EXPORT_RESULT_ATTRIBUTES:
+        setattr(results1, x, [f"{x}1"])
+        setattr(results2, x, [f"{x}2"])
+
+    results1 += results2
+    for x in EXPORT_RESULT_ATTRIBUTES:
+        assert getattr(results1, x) == [f"{x}1", f"{x}2"]
+
+    # exiftool_warning and exiftool_error are lists of tuples
+    results1 = ExportResults()
+    results2 = ExportResults()
+    results1.exiftool_warning = [("exiftool_warning1", "foo")]
+    results2.exiftool_warning = [("exiftool_warning2", "bar")]
+    results1.exiftool_error = [("exiftool_error1", "foo")]
+    results2.exiftool_error = [("exiftool_error2", "bar")]
+
+    results1 += results2
+
+    assert results1.exiftool_warning == [
+        ("exiftool_warning1", "foo"),
+        ("exiftool_warning2", "bar"),
+    ]
+    assert results1.exiftool_error == [
+        ("exiftool_error1", "foo"),
+        ("exiftool_error2", "bar"),
+    ]
+
+
+def test_all_files():
+    """ test ExportResults.all_files() """
+    results = ExportResults()
+    for x in EXPORT_RESULT_ATTRIBUTES:
+        setattr(results, x, [f"{x}1"])
+    results.exiftool_warning = [("exiftool_warning1", "foo")]
+    results.exiftool_error = [("exiftool_error1", "foo")]
+
+    assert sorted(results.all_files()) == sorted(
+        [f"{x}1" for x in EXPORT_RESULT_ATTRIBUTES]
+    )
+
+
+def test_str():
+    """ test ExportResults.__str__ """
+    results = ExportResults()
+    assert (
+        str(results)
+        == "ExportResults(exported=[],new=[],updated=[],skipped=[],exif_updated=[],touched=[],converted_to_jpeg=[],sidecar_json_written=[],sidecar_json_skipped=[],sidecar_xmp_written=[],sidecar_xmp_skipped=[],missing=[],error=[],exiftool_warning=[],exiftool_error=[])"
+    )
+