Proposed fix for #1703 timewarp pull-exif timezone fails to update Photos Timezone info when offset_seconds

osxphotos/cli/exportdb.py

@@ -59,12 +59,14 @@
 @click.option(
     "--check-signatures",
     is_flag=True,
-    help="Check signatures for all exported photos in the database to find signatures that don't match.",
+    help="Check signatures for all exported photos in the database to find signatures that don't match. "
+    "See also option --export-dir.",
 )
 @click.option(
     "--update-signatures",
     is_flag=True,
-    help="Update signatures for all exported photos in the database to match on-disk signatures.",
+    help="Update signatures for all exported photos in the database to match on-disk signatures. "
+    "See also option --export-dir.",
 )
 @click.option(
     "--touch-file",
@@ -268,6 +270,7 @@ def exportdb(
             info,
             last_run,
             last_export_dir,
+            migrate_photos_library,
             upgrade,
             repair,
             report,
@@ -323,47 +326,63 @@ def exportdb(
             )
             sys.exit(1)
 
-        try:
-            ExportDB(export_db, export_dir, create)
-        except Exception as e:
-            rich_echo_error(f"[error]Error: {e}[/error]")
-            sys.exit(1)
+        if not dry_run:
+            try:
+                ExportDB(export_db, export_dir, create)
+            except Exception as e:
+                rich_echo_error(f"[error]Error: {e}[/error]")
+                sys.exit(1)
+            else:
+                rich_echo(f"Created export database [filepath]{export_db}[/]")
+                sys.exit(0)
         else:
-            rich_echo(f"Created export database [filepath]{export_db}[/]")
+            rich_echo(f"[Dryrun] Created export database [filepath]{export_db}[/]")
             sys.exit(0)
 
     if check:
-        errors = sqlite_check_integrity(export_db)
-        if not errors:
-            rich_echo(f"Ok: [filepath]{export_db}[/]")
-            sys.exit(0)
+        if not dry_run:
+            errors = sqlite_check_integrity(export_db)
+            if not errors:
+                rich_echo(f"Ok: [filepath]{export_db}[/]")
+                sys.exit(0)
+            else:
+                rich_echo_error(f"[error]Errors: [filepath]{export_db}[/][/error]")
+                for error in errors:
+                    rich_echo_error(error)
+                sys.exit(1)
         else:
-            rich_echo_error(f"[error]Errors: [filepath]{export_db}[/][/error]")
-            for error in errors:
-                rich_echo_error(error)
-            sys.exit(1)
+            rich_echo(f"[Dryrun] Check [filepath]{export_db}[/]")
+            sys.exit(0)
 
     if repair:
-        try:
-            sqlite_repair_db(export_db)
-        except Exception as e:
-            rich_echo_error(f"[error]Error: {e}[/error]")
-            sys.exit(1)
+        if not dry_run:
+            try:
+                sqlite_repair_db(export_db)
+            except Exception as e:
+                rich_echo_error(f"[error]Error: {e}[/error]")
+                sys.exit(1)
+            else:
+                rich_echo(f"Ok: [filepath]{export_db}[/]")
+                sys.exit(0)
         else:
-            rich_echo(f"Ok: [filepath]{export_db}[/]")
+            rich_echo(f"[Dryrun] Repair [filepath]{export_db}[/]")
             sys.exit(0)
 
     if vacuum:
-        try:
-            start_size = pathlib.Path(export_db).stat().st_size
-            export_db_vacuum(export_db)
-        except Exception as e:
-            rich_echo_error(f"[error]Error: {e}[/error]")
-            sys.exit(1)
+        if not dry_run:
+            try:
+                start_size = pathlib.Path(export_db).stat().st_size
+                export_db_vacuum(export_db)
+            except Exception as e:
+                rich_echo_error(f"[error]Error: {e}[/error]")
+                sys.exit(1)
+            else:
+                rich_echo(
+                    f"Vacuumed {export_db}! [num]{start_size}[/] bytes -> [num]{pathlib.Path(export_db).stat().st_size}[/] bytes"
+                )
+                sys.exit(0)
         else:
-            rich_echo(
-                f"Vacuumed {export_db}! [num]{start_size}[/] bytes -> [num]{pathlib.Path(export_db).stat().st_size}[/] bytes"
-            )
+            rich_echo(f"[Dryrun] Vacuum [filepath]{export_db}[/]")
             sys.exit(0)
 
     if update_signatures:
@@ -417,13 +436,17 @@ def exportdb(
                 sys.exit(1)
 
     if save_config:
-        try:
-            export_db_save_config_to_file(export_db, save_config)
-        except Exception as e:
-            rich_echo_error(f"[error]Error: {e}[/error]")
-            sys.exit(1)
+        if not dry_run:
+            try:
+                export_db_save_config_to_file(export_db, save_config)
+            except Exception as e:
+                rich_echo_error(f"[error]Error: {e}[/error]")
+                sys.exit(1)
+            else:
+                rich_echo(f"Saved configuration to [filepath]{save_config}")
+                sys.exit(0)
         else:
-            rich_echo(f"Saved configuration to [filepath]{save_config}")
+            rich_echo(f"[Dryrun] Saved configuration to [filepath]{save_config}")
             sys.exit(0)
 
     if check_signatures:
@@ -561,21 +584,23 @@ def exportdb(
         exportdb = ExportDB(export_db, export_dir)
         for uuid in delete_uuid:
             rich_echo(f"Deleting uuid [uuid]{uuid}[/] from database.")
-            count = exportdb.delete_data_for_uuid(uuid)
-            rich_echo(
-                f"Deleted [num]{count}[/] {pluralize(count, 'record', 'records')}."
-            )
+            if not dry_run:
+                count = exportdb.delete_data_for_uuid(uuid)
+                rich_echo(
+                    f"Deleted [num]{count}[/] {pluralize(count, 'record', 'records')}."
+                )
         sys.exit(0)
 
     if delete_file:
         # delete information associated with a file from the export database
         exportdb = ExportDB(export_db, export_dir)
         for filepath in delete_file:
             rich_echo(f"Deleting file [filepath]{filepath}[/] from database.")
-            count = exportdb.delete_data_for_filepath(filepath)
-            rich_echo(
-                f"Deleted [num]{count}[/] {pluralize(count, 'record', 'records')}."
-            )
+            if not dry_run:
+                count = exportdb.delete_data_for_filepath(filepath)
+                rich_echo(
+                    f"Deleted [num]{count}[/] {pluralize(count, 'record', 'records')}."
+                )
             sys.exit(0)
 
     if report:
@@ -599,28 +624,40 @@ def exportdb(
         sys.exit(0)
 
     if upgrade:
-        exportdb = ExportDB(export_db, export_dir)
-        if upgraded := exportdb.was_upgraded:
-            rich_echo(
-                f"Upgraded export database [filepath]{export_db}[/] from version [num]{upgraded[0]}[/] to [num]{upgraded[1]}[/]"
-            )
+        if not dry_run:
+            exportdb = ExportDB(export_db, export_dir)
+            if upgraded := exportdb.was_upgraded:
+                rich_echo(
+                    f"Upgraded export database [filepath]{export_db}[/] from version [num]{upgraded[0]}[/] to [num]{upgraded[1]}[/]"
+                )
+            else:
+                rich_echo(
+                    f"Export database [filepath]{export_db}[/] is already at latest version [num]{OSXPHOTOS_EXPORTDB_VERSION}[/]"
+                )
         else:
+            # Does OSXPHOTOS_EXPORTDB_VERSION reflect the actual exportdb file version?
             rich_echo(
-                f"Export database [filepath]{export_db}[/] is already at latest version [num]{OSXPHOTOS_EXPORTDB_VERSION}[/]"
+                f"[Dryrun] Upgrading database [filepath]{export_db}[/]"
             )
         sys.exit(0)
 
     if sql:
-        exportdb = ExportDB(export_db, export_dir)
-        try:
-            c = exportdb._conn.cursor()
-            results = c.execute(sql)
-        except Exception as e:
-            rich_echo_error(f"[error]Error: {e}[/error]")
-            sys.exit(1)
+        if not dry_run:
+            exportdb = ExportDB(export_db, export_dir)
+            try:
+                c = exportdb._conn.cursor()
+                results = c.execute(sql)
+            except Exception as e:
+                rich_echo_error(f"[error]Error: {e}[/error]")
+                sys.exit(1)
+            else:
+                for row in results:
+                    print(row)
+                sys.exit(0)
         else:
-            for row in results:
-                print(row)
+            rich_echo(
+                f"[Dryrun] SQL_STATEMENT: [filepath]{sql}[/]"
+            )
             sys.exit(0)
 
     if migrate_photos_library:

osxphotos/cli/push_exif.py

@@ -635,10 +635,16 @@ def compare_location(photo: PhotoInfo, file_data: dict[str, Any]) -> str:
     """Compare location between Photos and original file for a single photo"""
     photo_latitude = photo.latitude
     photo_longitude = photo.longitude
-    exif_latitude = file_data.get("EXIF:GPSLatitude")
-    exif_longitude = file_data.get("EXIF:GPSLongitude")
-    exif_latitude_ref = file_data.get("EXIF:GPSLatitudeRef")
-    exif_longitude_ref = file_data.get("EXIF:GPSLongitudeRef")
+    if photo.isphoto:
+        exif_latitude = file_data.get("EXIF:GPSLatitude")
+        exif_longitude = file_data.get("EXIF:GPSLongitude")
+        exif_latitude_ref = file_data.get("EXIF:GPSLatitudeRef")
+        exif_longitude_ref = file_data.get("EXIF:GPSLongitudeRef")
+    elif photo.ismovie:
+        exif_coordinates = file_data.get('QuickTime:GPSCoordinates')
+        exif_latitude, exif_longitude = [float(x) for x in exif_coordinates.split()[:2]] if exif_coordinates else [ None, None]
+        exif_latitude_ref = None
+        exif_longitude_ref = None        
 
     if exif_longitude and exif_longitude_ref == "W":
         exif_longitude = -exif_longitude

osxphotos/cli/timewarp.py

@@ -449,6 +449,9 @@ def timewarp(
             "must be specified."
         )
 
+    if inspect and compare_exif:
+        raise click.UsageError("--inspect and --compare-exif are mutually exclusive.")
+
     if date and date_delta:
         raise click.UsageError("--date and --date-delta are mutually exclusive.")
 
@@ -620,11 +623,7 @@ def timewarp(
                 )
         for photo in photos:
             set_crash_data("photo", f"{photo.uuid} {photo.filename}")
-            diff_results = (
-                photocomp.compare_exif_no_markup(photo)
-                if plain
-                else photocomp.compare_exif_with_markup(photo)
-            )
+            diff_results = photocomp.timewarp_compare_exif(photo, plain)
 
             if not plain:
                 filename = (

osxphotos/compare_exif.py

@@ -87,79 +87,45 @@ def compare_exif(self, photo: Photo) -> List[str]:
 
         return [photos_date_str, photos_tz_str, exif_date, exif_offset]
 
-    def compare_exif_with_markup(self, photo: Photo) -> ExifDiff:
+    def timewarp_compare_exif(self, photo: Photo, plain: bool = False) -> ExifDiff:
         """Compare date/time/timezone in Photos to the exif data and return an ExifDiff named tuple;
-        adds rich markup to strings to show differences
+        optionally adds rich markup to strings to show differences.
 
         Args:
             photo (Photo): Photo object to compare
-        """
+            plain (bool): Flag to determine if plain (True) or markup (False) should be applied
+        """ 
+        def compare_values(photo_value: str, exif_value: str) -> tuple:
+            """Compare two values and return them with or without markup.
+
+            Affects nonlocal variable diff (from timewarp_compare_exif) with result.
+            """
+
+            nonlocal diff
+            if photo_value != exif_value:
+                diff = True
+                if not plain:
+                    return change(photo_value), change(exif_value)
+            else:
+                if not plain:
+                    return no_change(photo_value), no_change(exif_value)
+            return photo_value, exif_value
+
+        # Get values from comparison function
         photos_date, photos_tz, exif_date, exif_tz = self.compare_exif(photo)
         diff = False
-        photos_date, photos_time = photos_date.split(" ", 1)
-        try:
-            exif_date, exif_time = exif_date.split(" ", 1)
-        except ValueError:
-            exif_date = exif_date
-            exif_time = ""
-
-        if photos_date != exif_date:
-            photos_date = change(photos_date)
-            exif_date = change(exif_date)
-            diff = True
-        else:
-            photos_date = no_change(photos_date)
-            exif_date = no_change(exif_date)
-
-        if photos_time != exif_time:
-            photos_time = change(photos_time)
-            exif_time = change(exif_time)
-            diff = True
-        else:
-            photos_time = no_change(photos_time)
-            exif_time = no_change(exif_time)
-
-        if photos_tz != exif_tz:
-            photos_tz = change(photos_tz)
-            exif_tz = change(exif_tz)
-            diff = True
-        else:
-            photos_tz = no_change(photos_tz)
-            exif_tz = no_change(exif_tz)
-
-        return ExifDiff(
-            diff,
-            photos_date,
-            photos_time,
-            photos_tz,
-            exif_date,
-            exif_time,
-            exif_tz,
-        )
-
-    def compare_exif_no_markup(self, photo: Photo) -> ExifDiff:
-        """Compare date/time/timezone in Photos to the exif data and return an ExifDiff named tuple;
 
-        Args:
-            photo (Photo): Photo object to compare
-        """
-        photos_date, photos_tz, exif_date, exif_tz = self.compare_exif(photo)
-        diff = False
+        # Split date and time
         photos_date, photos_time = photos_date.split(" ", 1)
         try:
             exif_date, exif_time = exif_date.split(" ", 1)
         except ValueError:
-            exif_date = exif_date
-            exif_time = ""
-
-        if photos_date != exif_date:
-            diff = True
-
-        if photos_time != exif_time:
-            diff = True
+            exif_time = ""  # Handle missing time in exif_date
 
-        if photos_tz != exif_tz:
-            diff = True
+        # Compare dates, times, and timezones
+        photos_date, exif_date = compare_values(photos_date, exif_date)
+        photos_time, exif_time = compare_values(photos_time, exif_time)
+        photos_tz, exif_tz = compare_values(photos_tz, exif_tz)
 
         return ExifDiff(
             diff,
@@ -170,3 +136,4 @@ def compare_exif_no_markup(self, photo: Photo) -> ExifDiff:
             exif_time,
             exif_tz,
         )
+

osxphotos/exif_datetime_updater.py

@@ -178,7 +178,9 @@ def update_photos_from_exif(
             )
             return None
 
-        if dtinfo.offset_seconds:
+        print(f"{dtinfo=} {type(dtinfo)=} {dtinfo.offset_seconds=}")
+
+        if dtinfo.offset_seconds is not None:
             # update timezone then update date/time
             timezone = Timezone(dtinfo.offset_seconds)
             tzupdater = PhotoTimeZoneUpdater(