Completed implementation of --jpeg-ext, fixed --dry-run, closes #330, #346

CHANGELOG.md

@@ -4,6 +4,12 @@ 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.39.13](https://github.com/RhetTbull/osxphotos/compare/v0.39.12...v0.39.13)
+
+> 10 January 2021
+
+- Fixed leaky memory in PhotoKit, issue #276 [`db1947d`](https://github.com/RhetTbull/osxphotos/commit/db1947dd1e3d47a487eeb68a5ceb5f7098f1df10)
+
 #### [v0.39.12](https://github.com/RhetTbull/osxphotos/compare/v0.39.11...v0.39.12)
 
 > 9 January 2021

README.md

@@ -295,6 +295,10 @@ Options:
   --export-as-hardlink            Hardlink files instead of copying them.
                                   Cannot be used with --exiftool which creates
                                   copies of the files with embedded EXIF data.
+                                  Note: on APFS volumes, files are cloned when
+                                  exporting giving many of the same advantages
+                                  as hardlinks without having to use --export-
+                                  as-hardlink.
   --touch-file                    Sets the file's modification time to match
                                   photo date.
   --overwrite                     Overwrite existing files. Default behavior
@@ -489,6 +493,15 @@ Options:
                                   do not include an extension in the FILENAME
                                   template. See below for additional details
                                   on templating system.
+  --jpeg-ext EXTENSION            Specify file extension for JPEG files.
+                                  Photos uses .jpeg for edited images but many
+                                  images are imported with .jpg or .JPG which
+                                  can result in multiple different extensions
+                                  used for JPEG files upon export.  Use --jpg-
+                                  ext to specify a single extension to use for
+                                  all exported JPEG images. Valid values are
+                                  jpeg, jpg, JPEG, JPG; e.g. '--jpg-ext jpg'
+                                  to use '.jpg' for all JPEGs.
   --strip                         Optionally strip leading and trailing
                                   whitespace from any rendered templates. For
                                   example, if --filename template is "{title,}

osxphotos/__main__.py

@@ -47,6 +47,7 @@ from .path_utils import is_valid_filepath, sanitize_filename, sanitize_filepath
 from .photoinfo import ExportResults
 from .photokit import check_photokit_authorization, request_photokit_authorization
 from .phototemplate import TEMPLATE_SUBSTITUTIONS, TEMPLATE_SUBSTITUTIONS_MULTI_VALUED
+from .utils import get_preferred_uti_extension
 
 # global variable to control verbose output
 # set via --verbose/-V
@@ -1588,6 +1589,16 @@ def query(
     "File extension will be added automatically--do not include an extension in the FILENAME template. "
     "See below for additional details on templating system.",
 )
+@click.option(
+    "--jpeg-ext",
+    multiple=False,
+    metavar="EXTENSION",
+    type=click.Choice(["jpeg", "jpg", "JPEG", "JPG"], case_sensitive=True),
+    help="Specify file extension for JPEG files. Photos uses .jpeg for edited images but many images "
+    "are imported with .jpg or .JPG which can result in multiple different extensions used for JPEG files "
+    "upon export.  Use --jpg-ext to specify a single extension to use for all exported JPEG images. "
+    "Valid values are jpeg, jpg, JPEG, JPG; e.g. '--jpg-ext jpg' to use '.jpg' for all JPEGs.",
+)
 @click.option(
     "--strip",
     is_flag=True,
@@ -1759,6 +1770,7 @@ def export(
     has_raw,
     directory,
     filename_template,
+    jpeg_ext,
     strip,
     edited_suffix,
     original_suffix,
@@ -1898,6 +1910,7 @@ def export(
         has_raw = cfg.has_raw
         directory = cfg.directory
         filename_template = cfg.filename_template
+        jpeg_ext = cfg.jpeg_ext
         strip = cfg.strip
         edited_suffix = cfg.edited_suffix
         original_suffix = cfg.original_suffix
@@ -2265,6 +2278,7 @@ def export(
                     use_photokit=use_photokit,
                     exiftool_option=exiftool_option,
                     strip=strip,
+                    jpeg_ext=jpeg_ext,
                 )
                 results += export_results
 
@@ -2839,6 +2853,7 @@ def export_photo(
     use_photokit=False,
     exiftool_option=None,
     strip=False,
+    jpeg_ext=None,
 ):
     """Helper function for export that does the actual export
 
@@ -2876,6 +2891,7 @@ def export_photo(
         exiftool_option: optional list flags (e.g. ["-m", "-F"]) to pass to exiftool
         exiftool_merge_keywords: boolean; if True, merged keywords found in file's exif data (requires exiftool)
         exiftool_merge_persons: boolean; if True, merged persons found in file's exif data (requires exiftool)
+        jpeg_ext: if not None, specify the extension to use for all JPEG images on export
 
     Returns:
         list of path(s) of exported photo or None if photo was missing
@@ -2933,6 +2949,7 @@ def export_photo(
         photo, filename_template, original_name, strip=strip
     )
     for filename in filenames:
+        rendered_suffix = ""
         if original_suffix:
             try:
                 rendered_suffix, unmatched = photo.render_template(
@@ -2955,14 +2972,17 @@ def export_photo(
                 )
             rendered_suffix = rendered_suffix[0]
 
-            original_filename = pathlib.Path(filename)
-            original_filename = (
-                original_filename.parent
-                / f"{original_filename.stem}{rendered_suffix}{original_filename.suffix}"
-            )
-            original_filename = str(original_filename)
-        else:
-            original_filename = filename
+        original_filename = pathlib.Path(filename)
+        file_ext = (
+            "." + jpeg_ext
+            if jpeg_ext and photo.uti == "public.jpeg"
+            else original_filename.suffix
+        )
+        original_filename = (
+            original_filename.parent
+            / f"{original_filename.stem}{rendered_suffix}{file_ext}"
+        )
+        original_filename = str(original_filename)
 
         verbose_(
             f"Exporting {photo.original_filename} ({photo.filename}) as {original_filename}"
@@ -3046,6 +3066,7 @@ def export_photo(
                             use_photokit=use_photokit,
                             verbose=verbose_,
                             exiftool_flags=exiftool_option,
+                            jpeg_ext=jpeg_ext,
                         )
                         results += export_results
                         for warning_ in export_results.exiftool_warning:
@@ -3087,13 +3108,15 @@ def export_photo(
             # verify the photo has adjustments and valid path to avoid raising an exception
             if export_edited and photo.hasadjustments:
                 edited_filename = pathlib.Path(filename)
-                # check for correct edited suffix
-                if photo.path_edited is not None:
-                    edited_ext = pathlib.Path(photo.path_edited).suffix
-                else:
-                    # use filename suffix which might be wrong,
-                    # will be corrected by use_photos_export
-                    edited_ext = pathlib.Path(photo.filename).suffix
+                edited_ext = (
+                    "." + jpeg_ext
+                    if jpeg_ext and photo.uti_edited == "public.jpeg"
+                    else "." + get_preferred_uti_extension(photo.uti_edited)
+                    if photo.uti_edited
+                    else pathlib.Path(photo.path_edited).suffix
+                    if photo.path_edited
+                    else pathlib.Path(photo.filename).suffix
+                )
 
                 if edited_suffix:
                     try:
@@ -3128,7 +3151,9 @@ def export_photo(
                 )
                 if missing_edited:
                     space = " " if not verbose else ""
-                    verbose_(f"{space}Skipping missing edited photo for {edited_filename}")
+                    verbose_(
+                        f"{space}Skipping missing edited photo for {edited_filename}"
+                    )
                     results.missing.append(
                         str(pathlib.Path(dest_path) / edited_filename)
                     )
@@ -3140,7 +3165,7 @@ def export_photo(
                         f"{space}Skipping missing deleted photo {photo.original_filename} ({photo.uuid})"
                     )
                     results.missing.append(
-                        str(pathlib.Path(dest_path) / edited_filename )
+                        str(pathlib.Path(dest_path) / edited_filename)
                     )
 
                 else:
@@ -3173,6 +3198,7 @@ def export_photo(
                             use_photokit=use_photokit,
                             verbose=verbose_,
                             exiftool_flags=exiftool_option,
+                            jpeg_ext=jpeg_ext,
                         )
                         results += export_results_edited
                         for warning_ in export_results_edited.exiftool_warning:

osxphotos/_version.py

@@ -1,5 +1,3 @@
 """ version info """
 
-__version__ = "0.39.13"
-
-
+__version__ = "0.39.15"

osxphotos/fileutil.py

@@ -60,6 +60,11 @@ class FileUtilABC(ABC):
     def convert_to_jpeg(cls, src_file, dest_file, compression_quality=1.0):
         pass
 
+    @classmethod
+    @abstractmethod
+    def rename(cls, src, dest):
+        pass
+
 
 class FileUtilMacOS(FileUtilABC):
     """ Various file utilities """
@@ -201,6 +206,21 @@ class FileUtilMacOS(FileUtilABC):
             src_file, dest_file, compression_quality=compression_quality
         )
 
+    @classmethod
+    def rename(cls, src, dest):
+        """ Copy src to dest 
+
+        Args:
+            src: path to source file
+            dest: path to destination file
+        
+        Returns:
+            Name of renamed file (dest)
+        
+        """
+        os.rename(str(src), str(dest))
+        return dest
+
     @staticmethod
     def _sig(st):
         """ return tuple of (mode, size, mtime) of file based on os.stat
@@ -266,3 +286,7 @@ class FileUtilNoOp(FileUtil):
     @classmethod
     def convert_to_jpeg(cls, src_file, dest_file, compression_quality=1.0):
         cls.verbose(f"convert_to_jpeg: {src_file}, {dest_file}, {compression_quality}")
+
+    @classmethod
+    def rename(cls, src, dest):
+        cls.verbose(f"rename: {src}, {dest}")

osxphotos/photoinfo/_photoinfo_export.py

@@ -49,7 +49,7 @@ from ..photokit import (
     PhotoKitFetchFailed,
     PhotoLibrary,
 )
-from ..utils import dd_to_dms_str, findfiles, noop
+from ..utils import dd_to_dms_str, findfiles, noop, get_preferred_uti_extension
 
 
 class ExportError(Exception):
@@ -311,6 +311,34 @@ def _check_export_suffix(src, dest, edited):
     )
 
 
+# not a class method, don't import into PhotoInfo
+def rename_jpeg_files(files, jpeg_ext, fileutil):
+    """ rename any jpeg files in files so that extension matches jpeg_ext
+
+    Args:
+        files: list of file paths
+        jpeg_ext: extension to use for jpeg files found in files, e.g. "jpg"
+        fileutil: a FileUtil object
+    
+    Returns:
+        list of files with updated names
+
+    Note: If non-jpeg files found, they will be ignore and returned in the return list
+    """
+    jpeg_ext = "." + jpeg_ext
+    jpegs = [".jpeg", ".jpg"]
+    new_files = []
+    for file in files:
+        path = pathlib.Path(file)
+        if path.suffix.lower() in jpegs and path.suffix != jpeg_ext:
+            new_file = path.parent / (path.stem + jpeg_ext)
+            fileutil.rename(file, new_file)
+            new_files.append(new_file)
+        else:
+            new_files.append(file)
+    return new_files
+
+
 def export(
     self,
     dest,
@@ -437,6 +465,7 @@ def export2(
     exiftool_flags=None,
     merge_exif_keywords=False,
     merge_exif_persons=False,
+    jpeg_ext=None,
 ):
     """export photo, like export but with update and dry_run options
     dest: must be valid destination path or exception raised
@@ -488,6 +517,7 @@ def export2(
     exiftool_flags: optional list of flags to pass to exiftool when using exiftool option, e.g ["-m", "-F"]
     merge_exif_keywords: boolean; if True, merged keywords found in file's exif data (requires exiftool)
     merge_exif_persons: boolean; if True, merged persons found in file's exif data (requires exiftool)
+    jpeg_ext: if set, will use this value for extension on jpegs converted to jpeg with convert_to_jpeg; if not set, uses jpeg; do not include the leading "."
 
     Returns: ExportResults class 
         ExportResults has attributes: 
@@ -576,7 +606,8 @@ def export2(
     if convert_to_jpeg and self.isphoto and uti != "public.jpeg":
         # not a jpeg but will convert to jpeg upon export so fix file extension
         fname_new = pathlib.Path(fname)
-        fname = str(fname_new.parent / f"{fname_new.stem}.jpeg")
+        ext = "." + jpeg_ext if jpeg_ext else ".jpeg"
+        fname = str(fname_new.parent / f"{fname_new.stem}{ext}")
     else:
         # nothing to convert
         convert_to_jpeg = False
@@ -746,6 +777,8 @@ def export2(
                 )
                 all_results += results
     else:
+        # TODO: move this big if/else block to separate functions
+        # e.g. _export_with_photos_export or such
         # use_photo_export
         # export live_photo .mov file?
         live_photo = True if live_photo and self.live_photo else False
@@ -760,7 +793,10 @@ def export2(
             else:
                 # didn't get passed a filename, add _edited
                 filestem = f"{dest.stem}{edited_identifier}"
-                dest = dest.parent / f"{filestem}.jpeg"
+                uti = self.uti_edited if edited and self.uti_edited else self.uti
+                ext = get_preferred_uti_extension(uti)
+                dest = dest.parent / f"{filestem}{ext}"
+
             if use_photokit:
                 photolib = PhotoLibrary()
                 photo = None
@@ -783,13 +819,17 @@ def export2(
                             )
                         )
                 if photo:
-                    try:
-                        exported = photo.export(
-                            dest.parent, dest.name, version=PHOTOS_VERSION_CURRENT
-                        )
-                        all_results.exported.extend(exported)
-                    except Exception as e:
-                        all_results.error.append((str(dest), e))
+                    if not dry_run:
+                        try:
+                            exported = photo.export(
+                                dest.parent, dest.name, version=PHOTOS_VERSION_CURRENT
+                            )
+                            all_results.exported.extend(exported)
+                        except Exception as e:
+                            all_results.error.append((str(dest), e))
+                    else:
+                        # dry_run, don't actually export
+                        all_results.exported.append(str(dest))
             else:
                 try:
                     exported = _export_photo_uuid_applescript(
@@ -824,13 +864,17 @@ def export2(
                         photo = [p for p in bursts if p.uuid.startswith(self.uuid)]
                         photo = photo[0] if photo else None
                 if photo:
-                    try:
-                        exported = photo.export(
-                            dest.parent, dest.name, version=PHOTOS_VERSION_ORIGINAL
-                        )
-                        all_results.exported.extend(exported)
-                    except Exception as e:
-                        all_results.error.append((str(dest), e))
+                    if not dry_run:
+                        try:
+                            exported = photo.export(
+                                dest.parent, dest.name, version=PHOTOS_VERSION_ORIGINAL
+                            )
+                            all_results.exported.extend(exported)
+                        except Exception as e:
+                            all_results.error.append((str(dest), e))
+                    else:
+                        # dry_run, don't actually export
+                        all_results.exported.append(str(dest))
             else:
                 try:
                     exported = _export_photo_uuid_applescript(
@@ -848,6 +892,13 @@ def export2(
                 except ExportError as e:
                     all_results.error.append((str(dest), e))
         if all_results.exported:
+            if jpeg_ext:
+                # use_photos_export (both PhotoKit and AppleScript) don't use the
+                # file extension provided (instead they use extension for UTI)
+                # so if jpeg_ext is set, rename any non-conforming jpegs
+                all_results.exported = rename_jpeg_files(
+                    all_results.exported, jpeg_ext, fileutil
+                )
             if touch_file:
                 for exported_file in all_results.exported:
                     all_results.touched.append(exported_file)
@@ -856,9 +907,6 @@ def export2(
             if update:
                 all_results.new.extend(all_results.exported)
 
-        # else:
-        # all_results.error.append((str(dest), f"Error exporting photo {self.uuid} to {dest} with use_photos_export"))
-
     # export metadata
     sidecars = []
     sidecar_json_files_skipped = []
@@ -1766,3 +1814,4 @@ def _write_sidecar(self, filename, sidecar_str):
     f = open(filename, "w")
     f.write(sidecar_str)
     f.close()
+

tests/test_cli.py

@@ -565,6 +565,14 @@ UUID_NO_LIKES = [
     "1C1C8F1F-826B-4A24-B1CB-56628946A834",
 ]
 
+UUID_JPEGS_DICT = {
+    "4D521201-92AC-43E5-8F7C-59BC41C37A96": ["IMG_1997", "JPG"],
+    "E9BC5C36-7CD1-40A1-A72B-8B8FAC227D51": ["wedding", "jpg"],
+    "E2078879-A29C-4D6F-BACB-E3BBE6C3EB91": ["screenshot-really-a-png", "jpeg"],
+}
+
+UUID_HEIC = {"7783E8E6-9CAC-40F3-BE22-81FB7051C266": "IMG_3092"}
+
 
 def modify_file(filename):
     """ appends data to a file to modify it """
@@ -5238,3 +5246,77 @@ def test_export_xattr_template():
             assert sorted(md.keywords) == sorted(expected)
             assert md.comment == CLI_FINDER_TAGS[uuid]["XMP:Title"]
 
+
+def test_export_jpeg_ext():
+    """ test --jpeg-ext """
+    import glob
+    import os
+    import os.path
+    from osxphotos.__main__ import export
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        for uuid, fileinfo in UUID_JPEGS_DICT.items():
+            result = runner.invoke(
+                export, [os.path.join(cwd, PHOTOS_DB_15_7), ".", "-V", "--uuid", uuid]
+            )
+            assert result.exit_code == 0
+            files = glob.glob("*")
+            filename, ext = fileinfo
+            assert f"{filename}.{ext}" in files
+
+    for jpeg_ext in ["jpg", "JPG", "jpeg", "JPEG"]:
+        with runner.isolated_filesystem():
+            for uuid, fileinfo in UUID_JPEGS_DICT.items():
+                result = runner.invoke(
+                    export,
+                    [
+                        os.path.join(cwd, PHOTOS_DB_15_7),
+                        ".",
+                        "-V",
+                        "--uuid",
+                        uuid,
+                        "--jpeg-ext",
+                        jpeg_ext,
+                    ],
+                )
+                assert result.exit_code == 0
+                files = glob.glob("*")
+                filename, ext = fileinfo
+                assert f"{filename}.{jpeg_ext}" in files
+
+
+@pytest.mark.skipif(
+    "OSXPHOTOS_TEST_CONVERT" not in os.environ,
+    reason="Skip if running in Github actions, no GPU.",
+)
+def test_export_jpeg_ext_convert_to_jpeg():
+    """ test --jpeg-ext with --convert-to-jpeg """
+    import glob
+    import os
+    import os.path
+    from osxphotos.__main__ import export
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        for uuid, filename in UUID_HEIC.items():
+            result = runner.invoke(
+                export,
+                [
+                    os.path.join(cwd, PHOTOS_DB_15_7),
+                    ".",
+                    "-V",
+                    "--uuid",
+                    uuid,
+                    "--convert-to-jpeg",
+                    "--jpeg-ext",
+                    "jpg",
+                ],
+            )
+            assert result.exit_code == 0
+            files = glob.glob("*")
+            assert f"{filename}.jpg" in files

tests/test_fileutil.py

@@ -107,3 +107,21 @@ def test_convert_to_jpeg_quality():
         assert FileUtil.convert_to_jpeg(imgfile, outfile, compression_quality=0.1)
         assert outfile.is_file()
         assert outfile.stat().st_size < 1000000
+
+
+def test_rename_file():
+    # rename file with valid src, dest
+    import pathlib
+    import tempfile
+    from osxphotos.fileutil import FileUtil
+
+    temp_dir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    src = "tests/test-images/wedding.jpg"
+    dest = f"{temp_dir.name}/foo.jpg"
+    dest2 = f"{temp_dir.name}/bar.jpg"
+    FileUtil.copy(src, dest)
+    result = FileUtil.rename(dest, dest2)
+    assert result
+    assert pathlib.Path(dest2).exists()
+    assert not pathlib.Path(dest).exists()
+

utils/README.md

@@ -0,0 +1,18 @@
+# Utils
+
+These are various utilities used in my development workflow. They may or may not be useful to you if you're working on osxphotos.  If using the AppleScripts to get data from Photos, I highly recommend the excellent [FastScripts](https://redsweater.com/fastscripts/) from Red Sweater Software.
+
+## Files
+
+|File | Description |
+|-----|-------------|
+|build_help_table.py| Builds the template substitutions table used in main README.md |
+|check_uuid.py| Use with output file created by dump_photo_info.scpt to check ouput of osxphotos vs what Photos reports|
+|copy_uuid_to_clipboard.applescript| Copy UUID of selected photo in Photos to the Clipboard|
+|dump_photo_info.applescript| Dumps UUID and other info about every photo in Photos.app to a test file; see check_uuid.py|
+|dump_photo_info.scpt| Compiled version of dump_photo_info.applescript|
+|gen_face_test_data.py| Generate test data for test_faceinfo.py|
+|generate_search_info_test_data.py | Create the test data needed for test_search_info_10_15_7.py|
+|get_photo_info.applescript| Displays  UUID and other info about selected photos, useful for debugging|
+|get_photo_info.scpt| Compiled version of above|
+|write_uuid_to_file.applescript| Writes the UUIDs of selected images in Photos to a text file; can generate input for --uuid-from-file|

utils/copy_uuid_to_clipboard.applescript

@@ -0,0 +1,20 @@
+-- Copies UUID of selected photo to the clipboard, if more than one selection, copies uuid from the last item
+-- Useful for debugging with osxphotos
+
+
+tell application "Photos"
+	set uuid to ""
+	set theSelection to selection
+	repeat with theItem in theSelection
+		set uuid to ((id of theItem) as text)
+		set oldDelimiter to AppleScript's text item delimiters
+		set AppleScript's text item delimiters to "/"
+		set theTextItems to every text item of uuid
+		set uuid to first item of theTextItems
+		set AppleScript's text item delimiters to oldDelimiter
+	end repeat
+	set the clipboard to uuid
+	
+end tell
+
+