Fix for bug in handling of deleted albums to address issue #156

CHANGELOG.md

@@ -4,6 +4,13 @@ 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.29.9](https://github.com/RhetTbull/osxphotos/compare/v0.29.8...v0.29.9)
+
+> 31 May 2020
+
+- Added --filename to CLI, closes #89 [`#89`](https://github.com/RhetTbull/osxphotos/issues/89)
+- Updated CHANGELOG.md [`d47fd46`](https://github.com/RhetTbull/osxphotos/commit/d47fd46a21881bea86d1bc624c6027e2cbe08d9c)
+
 #### [v0.29.8](https://github.com/RhetTbull/osxphotos/compare/v0.29.5...v0.29.8)
 
 > 31 May 2020

examples/photos_repl.py

@@ -20,11 +20,7 @@ from osxphotos.__main__ import get_photos_db, _list_libraries
 def main():
     db = None
 
-    if len(sys.argv) > 1:
-        db = sys.argv[1]
-    else:
-        db = get_photos_db()
-
+    db = sys.argv[1] if len(sys.argv) > 1 else get_photos_db()
     if db:
         print("loading database")
         tic = time.perf_counter()

osxphotos/__main__.py

@@ -534,10 +534,7 @@ def info(ctx, cli_obj, db, json_, photos_library):
         return
 
     pdb = osxphotos.PhotosDB(dbfile=db)
-    info = {}
-    info["database_path"] = pdb.db_path
-    info["database_version"] = pdb.db_version
-
+    info = {"database_path": pdb.db_path, "database_version": pdb.db_version}
     photos = pdb.photos()
     not_shared_photos = [p for p in photos if not p.shared]
     info["photo_count"] = len(not_shared_photos)
@@ -1175,7 +1172,7 @@ def export(
         (export_as_hardlink, exiftool),
         (any(place), no_place),
     ]
-    if any([all(bb) for bb in exclusive]):
+    if any(all(bb) for bb in exclusive):
         click.echo("Incompatible export options", err=True)
         click.echo(cli.commands["export"].get_help(ctx), err=True)
         return
@@ -1186,11 +1183,6 @@ def export(
         not x for x in [skip_edited, skip_bursts, skip_live, skip_raw]
     ]
 
-    # though the command line option is current_name, internally all processing
-    # logic uses original_name which is the boolean inverse of current_name
-    # because the original code used --original-name as an option
-    original_name = not current_name
-
     # verify exiftool installed an in path
     if exiftool:
         try:
@@ -1283,10 +1275,6 @@ def export(
     )
 
     results_exported = []
-    results_new = []
-    results_updated = []
-    results_skipped = []
-    results_exif_updated = []
     if photos:
         if export_bursts:
             # add the burst_photos to the export set
@@ -1300,7 +1288,50 @@ def export(
         photo_str = "photos" if num_photos > 1 else "photo"
         click.echo(f"Exporting {num_photos} {photo_str} to {dest}...")
         start_time = time.perf_counter()
-        if not verbose_:
+        # though the command line option is current_name, internally all processing
+        # logic uses original_name which is the boolean inverse of current_name
+        # because the original code used --original-name as an option
+        original_name = not current_name
+
+        results_new = []
+        results_updated = []
+        results_skipped = []
+        results_exif_updated = []
+        if verbose_:
+            for p in photos:
+                results = export_photo(
+                    photo=p,
+                    dest=dest,
+                    verbose_=verbose_,
+                    export_by_date=export_by_date,
+                    sidecar=sidecar,
+                    update=update,
+                    export_as_hardlink=export_as_hardlink,
+                    overwrite=overwrite,
+                    export_edited=export_edited,
+                    original_name=original_name,
+                    export_live=export_live,
+                    download_missing=download_missing,
+                    exiftool=exiftool,
+                    directory=directory,
+                    filename_template=filename_template,
+                    no_extended_attributes=no_extended_attributes,
+                    export_raw=export_raw,
+                    album_keyword=album_keyword,
+                    person_keyword=person_keyword,
+                    keyword_template=keyword_template,
+                    export_db=export_db,
+                    fileutil=fileutil,
+                    dry_run=dry_run,
+                    edited_suffix=edited_suffix,
+                )
+                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)
+
+        else:
             # show progress bar
             with click.progressbar(photos) as bar:
                 for p in bar:
@@ -1335,47 +1366,9 @@ def export(
                     results_updated.extend(results.updated)
                     results_skipped.extend(results.skipped)
                     results_exif_updated.extend(results.exif_updated)
-        else:
-            for p in photos:
-                results = export_photo(
-                    photo=p,
-                    dest=dest,
-                    verbose_=verbose_,
-                    export_by_date=export_by_date,
-                    sidecar=sidecar,
-                    update=update,
-                    export_as_hardlink=export_as_hardlink,
-                    overwrite=overwrite,
-                    export_edited=export_edited,
-                    original_name=original_name,
-                    export_live=export_live,
-                    download_missing=download_missing,
-                    exiftool=exiftool,
-                    directory=directory,
-                    filename_template=filename_template,
-                    no_extended_attributes=no_extended_attributes,
-                    export_raw=export_raw,
-                    album_keyword=album_keyword,
-                    person_keyword=person_keyword,
-                    keyword_template=keyword_template,
-                    export_db=export_db,
-                    fileutil=fileutil,
-                    dry_run=dry_run,
-                    edited_suffix=edited_suffix,
-                )
-                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)
-
         stop_time = time.perf_counter()
         # print summary results
-        if not update:
-            photo_str = "photos" if len(results_exported) != 1 else "photo"
-            click.echo(f"Exported: {len(results_exported)} {photo_str}")
-            click.echo(f"Elapsed time: {stop_time-start_time} seconds")
-        else:
+        if update:
             photo_str_new = "photos" if len(results_new) != 1 else "photo"
             photo_str_updated = "photos" if len(results_new) != 1 else "photo"
             photo_str_skipped = "photos" if len(results_skipped) != 1 else "photo"
@@ -1388,8 +1381,10 @@ def export(
                 + f"skipped: {len(results_skipped)} {photo_str_skipped}, "
                 + f"updated EXIF data: {len(results_exif_updated)} {photo_str_exif_updated}"
             )
-            click.echo(f"Elapsed time: {stop_time-start_time} seconds")
-
+        else:
+            photo_str = "photos" if len(results_exported) != 1 else "photo"
+            click.echo(f"Exported: {len(results_exported)} {photo_str}")
+        click.echo(f"Elapsed time: {stop_time-start_time} seconds")
     else:
         click.echo("Did not find any photos to export")
 
@@ -1412,8 +1407,8 @@ def help(ctx, topic, **kw):
 
 
 def print_photo_info(photos, json=False):
+    dump = []
     if json:
-        dump = []
         for p in photos:
             dump.append(p.json())
         click.echo(f"[{', '.join(dump)}]")
@@ -1422,7 +1417,6 @@ def print_photo_info(photos, json=False):
         csv_writer = csv.writer(
             sys.stdout, delimiter=",", quotechar='"', quoting=csv.QUOTE_MINIMAL
         )
-        dump = []
         # add headers
         dump.append(
             [
@@ -1990,10 +1984,7 @@ def get_filenames_from_template(photo, filename_template, original_name):
             )
         filenames = [f"{file_}{photo_ext}" for file_ in filenames]
     else:
-        if original_name:
-            filenames = [photo.original_filename]
-        else:
-            filenames = [photo.filename]
+        filenames = [photo.original_filename] if original_name else [photo.filename]
     return filenames
 
 
@@ -2019,13 +2010,18 @@ def get_dirnames_from_template(photo, directory, export_by_date, dest, dry_run):
         dest_path = os.path.join(
             dest, date_created.year, date_created.mm, date_created.dd
         )
-        if not dry_run and not os.path.isdir(dest_path):
+        if not (dry_run or os.path.isdir(dest_path)):
             os.makedirs(dest_path)
         dest_paths = [dest_path]
     elif directory:
         # got a directory template, render it and check results are valid
         dirnames, unmatched = photo.render_template(directory)
-        if not dirnames or unmatched:
+        if not dirnames:
+            raise click.BadOptionUsage(
+                "directory",
+                f"Invalid template '{directory}': results={dirnames} unmatched={unmatched}",
+            )
+        elif unmatched:
             raise click.BadOptionUsage(
                 "directory",
                 f"Invalid template '{directory}': results={dirnames} unmatched={unmatched}",
@@ -2036,7 +2032,7 @@ def get_dirnames_from_template(photo, directory, export_by_date, dest, dry_run):
             dest_path = os.path.join(dest, dirname)
             if not is_valid_filepath(dest_path, platform="auto"):
                 raise ValueError(f"Invalid file path: '{dest_path}'")
-            if not dry_run and not os.path.isdir(dest_path):
+            if not (dry_run or os.path.isdir(dest_path)):
                 os.makedirs(dest_path)
             dest_paths.append(dest_path)
     else:

osxphotos/_export_db.py

@@ -445,10 +445,7 @@ class ExportDB(ExportDB_ABC):
         dt = datetime.datetime.utcnow().isoformat()
         python_path = sys.executable
         cmd = sys.argv[0]
-        if len(sys.argv) > 1:
-            args = " ".join(sys.argv[1:])
-        else:
-            args = ""
+        args = " ".join(sys.argv[1:]) if len(sys.argv) > 1 else ""
         cwd = os.getcwd()
         conn = self._conn
         try:

osxphotos/_version.py

@@ -1,3 +1,3 @@
 """ version info """
 
-__version__ = "0.29.9"
+__version__ = "0.29.12"

osxphotos/albuminfo.py

@@ -62,10 +62,7 @@ class AlbumInfo:
         try:
             return self._folder_names
         except AttributeError:
-            if self._db._db_version <= _PHOTOS_4_VERSION:
-                self._folder_names = self._db._album_folder_hierarchy_list(self._uuid)
-            else:
-                self._folder_names = self._db._album_folder_hierarchy_list(self._uuid)
+            self._folder_names = self._db._album_folder_hierarchy_list(self._uuid)
             return self._folder_names
 
     @property

osxphotos/datetime_formatter.py

@@ -12,53 +12,44 @@ class DateTimeFormatter:
     @property
     def date(self):
         """ ISO date in form 2020-03-22 """
-        date = self.dt.date().isoformat()
-        return date
+        return self.dt.date().isoformat()
 
     @property
     def year(self):
         """ 4 digit year """
-        year = f"{self.dt.year}"
-        return year
+        return f"{self.dt.year}"
 
     @property
     def yy(self):
         """ 2 digit year """
-        yy = f"{self.dt.strftime('%y')}"
-        return yy
+        return f"{self.dt.strftime('%y')}"
 
     @property
     def mm(self):
         """ 2 digit month """
-        mm = f"{self.dt.strftime('%m')}"
-        return mm
+        return f"{self.dt.strftime('%m')}"
 
     @property
     def month(self):
         """ Month as locale's full name """
-        month = f"{self.dt.strftime('%B')}"
-        return month
+        return f"{self.dt.strftime('%B')}"
 
     @property
     def mon(self):
         """ Month as locale's abbreviated name """
-        mon = f"{self.dt.strftime('%b')}"
-        return mon
+        return f"{self.dt.strftime('%b')}"
 
     @property
     def dd(self):
         """ 2-digit day of the month """
-        dd = f"{self.dt.strftime('%d')}"
-        return dd
+        return f"{self.dt.strftime('%d')}"
 
     @property
     def dow(self):
         """ Day of week as locale's name """
-        dow = f"{self.dt.strftime('%A')}"
-        return dow
+        return f"{self.dt.strftime('%A')}"
 
     @property
     def doy(self):
         """ Julian day of year starting from 001 """
-        doy = f"{self.dt.strftime('%j')}"
-        return doy
+        return f"{self.dt.strftime('%j')}"

osxphotos/exiftool.py

@@ -59,11 +59,7 @@ class _ExifToolProc:
                 )
             return
 
-        if exiftool:
-            self._exiftool = exiftool
-        else:
-            self._exiftool = get_exiftool_path()
-
+        self._exiftool = exiftool if exiftool else get_exiftool_path()
         self._process_running = False
         self._start_proc()
 
@@ -156,8 +152,7 @@ class ExifTool:
 
         if value is None:
             value = ""
-        command = []
-        command.append(f"-{tag}={value}")
+        command = [f"-{tag}={value}"]
         if self.overwrite:
             command.append("-overwrite_original")
         self.run_commands(*command)
@@ -193,7 +188,7 @@ class ExifTool:
             no_file: (bool) do not pass the filename to exiftool (default=False)
                      by default, all commands will be run against self.file
                      use no_file=True to run a command without passing the filename """
-        if not hasattr(self, "_process") or not self._process:
+        if not (hasattr(self, "_process") and self._process):
             raise ValueError("exiftool process is not running")
 
         if not commands:
@@ -245,8 +240,7 @@ class ExifTool:
 
     def json(self):
         """ returns JSON string containing all EXIF tags and values from exiftool """
-        json_str = self.run_commands("-json")
-        return json_str
+        return self.run_commands("-json")
 
     def _read_exif(self):
         """ read exif data from file """
@@ -254,5 +248,4 @@ class ExifTool:
         self.data = {k: v for k, v in data.items()}
 
     def __str__(self):
-        str_ = f"file: {self.file}\nexiftool: {self._exiftoolproc._exiftool}"
-        return str_
+        return f"file: {self.file}\nexiftool: {self._exiftoolproc._exiftool}"

osxphotos/fileutil.py

@@ -11,6 +11,7 @@ from abc import ABC, abstractmethod
 
 class FileUtilABC(ABC):
     """ Abstract base class for FileUtil """
+
     @classmethod
     @abstractmethod
     def hardlink(cls, src, dest):
@@ -39,6 +40,7 @@ class FileUtilABC(ABC):
 
 class FileUtilMacOS(FileUtilABC):
     """ Various file utilities """
+
     @classmethod
     def hardlink(cls, src, dest):
         """ Hardlinks a file from src path to dest path 
@@ -119,9 +121,7 @@ class FileUtilMacOS(FileUtilABC):
 
         if s1[0] != stat.S_IFREG or s2[0] != stat.S_IFREG:
             return False
-        if s1 == s2:
-            return True
-        return False
+        return s1 == s2
 
     @classmethod
     def file_sig(cls, f1):
@@ -135,14 +135,17 @@ class FileUtilMacOS(FileUtilABC):
 
 class FileUtil(FileUtilMacOS):
     """ Various file utilities """
+
     pass
 
+
 class FileUtilNoOp(FileUtil):
     """ No-Op implementation of FileUtil for testing / dry-run mode
         all methods with exception of cmp_sig and file_cmp are no-op
         cmp_sig functions as FileUtil.cmp_sig does
         file_cmp returns mock data
     """
+
     @staticmethod
     def noop(*args):
         pass
@@ -155,7 +158,7 @@ class FileUtilNoOp(FileUtil):
                 cls.verbose = verbose
             else:
                 raise ValueError(f"verbose {verbose} not callable")
-        return super(FileUtilNoOp, cls).__new__(cls) 
+        return super(FileUtilNoOp, cls).__new__(cls)
 
     @classmethod
     def hardlink(cls, src, dest):
@@ -164,7 +167,7 @@ class FileUtilNoOp(FileUtil):
     @classmethod
     def copy(cls, src, dest, norsrc=False):
         cls.verbose(f"copy: {src} {dest}")
-        
+
     @classmethod
     def unlink(cls, dest):
         cls.verbose(f"unlink: {dest}")

osxphotos/photoinfo/_photoinfo_export.py

@@ -156,6 +156,46 @@ def _export_photo_uuid_applescript(
         return None
 
 
+# _check_export_suffix is not a class method, don't import this into PhotoInfo
+def _check_export_suffix(src, dest, edited):
+    """Helper function for exporting photos to check file extensions of destination path.
+    
+    Checks that dst file extension is appropriate for the src.
+    If edited=True, will use src file extension of ".jpeg" if None provided for src.
+    
+    Args:
+        src: path to source file or None.
+        dest: path to destination file.
+        edited: set to True if exporting an edited photo.
+
+    Returns:
+        True if src and dest extensions are OK, else False.
+
+    Raises:
+        ValueError if edited is False and src is None
+    """
+
+    # check extension of destination
+    if src is not None:
+        # use suffix from edited file
+        actual_suffix = pathlib.Path(src).suffix
+    elif edited:
+        # use .jpeg as that's probably correct
+        actual_suffix = ".jpeg"
+    else:
+        raise ValueError("src must not be None if edited=False")
+
+    # Photo's often converts .JPG to .jpeg or .tif to .tiff on import
+    dest_ext = dest.suffix.lower()
+    actual_ext = actual_suffix.lower()
+    suffixes = sorted([dest_ext, actual_ext])
+    return (
+        dest_ext == actual_ext
+        or suffixes == [".jpeg", ".jpg"]
+        or suffixes == [".tif", ".tiff"]
+    )
+
+
 def export(
     self,
     dest,
@@ -182,8 +222,11 @@ def export(
                     **NOTE**: if provided, user must ensure file extension (suffix) is correct. 
                     For example, if photo is .CR2 file, edited image may be .jpeg.  
                     If you provide an extension different than what the actual file is, 
-                    export will print a warning but will happily export the photo using the 
-                    incorrect file extension.  e.g. to get the extension of the edited photo, 
+                    export will print a warning but will export the photo using the 
+                    incorrect file extension (unless use_photos_export is true, in which case export will
+                    use the extension provided by Photos upon export; in this case, an incorrect extension is
+                    silently ignored).
+                    e.g. to get the extension of the edited photo, 
                     reference PhotoInfo.path_edited
         edited: (boolean, default=False); if True will export the edited version of the photo 
                 (or raise exception if no edited version) 
@@ -260,13 +303,16 @@ def export2(
     dry_run=False,
 ):
     """ export photo, like export but with update and dry_run options
-        dest: must be valid destination path (or exception raised) 
+        dest: must be valid destination path or exception raised 
         filename: (optional): name of exported picture; if not provided, will use current filename 
                     **NOTE**: if provided, user must ensure file extension (suffix) is correct. 
                     For example, if photo is .CR2 file, edited image may be .jpeg.  
                     If you provide an extension different than what the actual file is, 
-                    export will print a warning but will happily export the photo using the 
-                    incorrect file extension.  e.g. to get the extension of the edited photo, 
+                    export will print a warning but will export the photo using the 
+                    incorrect file extension (unless use_photos_export is true, in which case export will
+                    use the extension provided by Photos upon export; in this case, an incorrect extension is
+                    silently ignored).
+                    e.g. to get the extension of the edited photo, 
                     reference PhotoInfo.path_edited
         edited: (boolean, default=False); if True will export the edited version of the photo 
                 (or raise exception if no edited version) 
@@ -370,27 +416,6 @@ def export2(
     fname = pathlib.Path(fname)
     dest = dest / fname
 
-    # check extension of destination
-    if edited and self.path_edited is not None:
-        # use suffix from edited file
-        actual_suffix = pathlib.Path(self.path_edited).suffix
-    elif edited:
-        # use .jpeg as that's probably correct
-        # if edited and path_edited is None, will raise FileNotFoundError below
-        # unless use_photos_export is True
-        actual_suffix = ".jpeg"
-    else:
-        # use suffix from the non-edited file
-        actual_suffix = pathlib.Path(self.filename).suffix
-
-    # warn if suffixes don't match but ignore .JPG / .jpeg as
-    # Photo's often converts .JPG to .jpeg
-    suffixes = sorted([x.lower() for x in [dest.suffix, actual_suffix]])
-    if dest.suffix.lower() != actual_suffix.lower() and suffixes != [".jpeg", ".jpg"]:
-        logging.warning(
-            f"Invalid destination suffix: {dest.suffix}, should be {actual_suffix}"
-        )
-
     # check to see if file exists and if so, add (1), (2), etc until we find one that works
     # Photos checks the stem and adds (1), (2), etc which avoids collision with sidecars
     # e.g. exporting sidecar for file1.png and file1.jpeg
@@ -438,6 +463,13 @@ def export2(
         if not os.path.isfile(src):
             raise FileNotFoundError(f"{src} does not appear to exist")
 
+        if not _check_export_suffix(src, dest, edited):
+            logging.warning(
+                f"Invalid destination suffix: {dest.suffix} for {self.path}, "
+                + f"edited={edited}, path_edited={self.path_edited}, "
+                + f"original_filename={self.original_filename}, filename={self.filename}"
+            )
+
         logging.debug(
             f"exporting {src} to {dest}, overwrite={overwrite}, increment={increment}, dest exists: {dest.exists()}"
         )
@@ -757,6 +789,8 @@ def _export_photo(
         action depending on update, overwrite
         Assumes destination is the right destination (e.g. UUID matches)
         sets UUID and JSON info foo exported file using set_uuid_for_file, set_inf_for_uuido
+    
+    Args:
         src: src path (string)
         dest: dest path (pathlib.Path)
         update: bool
@@ -766,7 +800,9 @@ def _export_photo(
         export_as_hardlink: bool
         exiftool: bool
         fileutil: FileUtil class that conforms to fileutil.FileUtilABC
-        Returns: ExportResults
+
+    Returns:
+        ExportResults
     """
 
     exported_files = []
@@ -1153,7 +1189,7 @@ def _xmp_sidecar(
 def _write_sidecar(self, filename, sidecar_str):
     """ write sidecar_str to filename
         used for exporting sidecar info """
-    if not filename and not sidecar_str:
+    if not (filename or sidecar_str):
         raise (
             ValueError(
                 f"filename {filename} and sidecar_str {sidecar_str} must not be None"

osxphotos/photosdb/photosdb.py

@@ -294,18 +294,17 @@ class PhotosDB:
     @property
     def keywords_as_dict(self):
         """ return keywords as dict of keyword, count in reverse sorted order (descending) """
-        keywords = {}
-        for k in self._dbkeywords_keyword.keys():
-            keywords[k] = len(self._dbkeywords_keyword[k])
+        keywords = {
+            k: len(self._dbkeywords_keyword[k]) for k in self._dbkeywords_keyword.keys()
+        }
+
         keywords = dict(sorted(keywords.items(), key=lambda kv: kv[1], reverse=True))
         return keywords
 
     @property
     def persons_as_dict(self):
         """ return persons as dict of person, count in reverse sorted order (descending) """
-        persons = {}
-        for k in self._dbfaces_person.keys():
-            persons[k] = len(self._dbfaces_person[k])
+        persons = {k: len(self._dbfaces_person[k]) for k in self._dbfaces_person.keys()}
         persons = dict(sorted(persons.items(), key=lambda kv: kv[1], reverse=True))
         return persons
 
@@ -413,13 +412,12 @@ class PhotosDB:
     def album_info(self):
         """ return list of AlbumInfo objects for each album in the photos database """
 
-        albums = [
+        return [
             AlbumInfo(db=self, uuid=album)
             for album in self._dbalbums_album.keys()
             if self._dbalbum_details[album]["cloudownerhashedpersonid"] is None
             and not self._dbalbum_details[album]["intrash"]
         ]
-        return albums
 
     @property
     def album_info_shared(self):
@@ -433,13 +431,12 @@ class PhotosDB:
             )
             return []
 
-        albums_shared = [
+        return [
             AlbumInfo(db=self, uuid=album)
             for album in self._dbalbums_album.keys()
             if self._dbalbum_details[album]["cloudownerhashedpersonid"] is not None
             and not self._dbalbum_details[album]["intrash"]
         ]
-        return albums_shared
 
     @property
     def albums(self):
@@ -2136,7 +2133,11 @@ class PhotosDB:
                     if album in self._dbalbum_titles:
                         title_set = set()
                         for album_id in self._dbalbum_titles[album]:
-                            title_set.update(self._dbalbums_album[album_id])
+                            try:
+                                title_set.update(self._dbalbums_album[album_id])
+                            except KeyError:
+                                # an empty album will be in _dbalbum_titles but not _dbalbums_album
+                                pass
                         album_set.update(title_set)
                     else:
                         logging.debug(f"Could not find album '{album}' in database")

osxphotos/phototemplate.py

@@ -1,5 +1,6 @@
 """ Custom template system for osxphotos (implemented in PhotoInfo.render_template) """
 
+
 # Rolled my own template system because:
 # 1. Needed to handle multiple values (e.g. album, keyword)
 # 2. Needed to handle default values if template not found
@@ -8,7 +9,6 @@
 # 4. Couldn't figure out how to do #1 and #2 with str.format()
 #
 # This code isn't elegant but it seems to work well.  PRs gladly accepted.
-
 import locale
 import os
 import re
@@ -71,7 +71,7 @@ TEMPLATE_SUBSTITUTIONS_MULTI_VALUED = {
 # Just the multi-valued substitution names without the braces
 MULTI_VALUE_SUBSTITUTIONS = [
     field.replace("{", "").replace("}", "")
-    for field in TEMPLATE_SUBSTITUTIONS_MULTI_VALUED.keys()
+    for field in TEMPLATE_SUBSTITUTIONS_MULTI_VALUED
 ]
 
 
@@ -234,172 +234,178 @@ class PhotoTemplate:
         """
 
         # must be a valid keyword
-        if field =="name":
+        if field == "name":
             return pathlib.Path(self.photo.filename).stem
 
-        if field =="original_name":
+        if field == "original_name":
             return pathlib.Path(self.photo.original_filename).stem
 
-        if field =="title":
+        if field == "title":
             return self.photo.title
 
-        if field =="descr":
+        if field == "descr":
             return self.photo.description
 
-        if field =="created.date":
+        if field == "created.date":
             return DateTimeFormatter(self.photo.date).date
 
-        if field =="created.year":
+        if field == "created.year":
             return DateTimeFormatter(self.photo.date).year
 
-        if field =="created.yy":
+        if field == "created.yy":
             return DateTimeFormatter(self.photo.date).yy
 
-        if field =="created.mm":
+        if field == "created.mm":
             return DateTimeFormatter(self.photo.date).mm
 
-        if field =="created.month":
+        if field == "created.month":
             return DateTimeFormatter(self.photo.date).month
 
-        if field =="created.mon":
+        if field == "created.mon":
             return DateTimeFormatter(self.photo.date).mon
 
-        if field =="created.dd":
+        if field == "created.dd":
             return DateTimeFormatter(self.photo.date).dd
 
-        if field =="created.dow":
+        if field == "created.dow":
             return DateTimeFormatter(self.photo.date).dow
 
-        if field =="created.doy":
+        if field == "created.doy":
             return DateTimeFormatter(self.photo.date).doy
 
-        if field =="modified.date":
+        if field == "modified.date":
             return (
                 DateTimeFormatter(self.photo.date_modified).date
                 if self.photo.date_modified
                 else None
             )
 
-        if field =="modified.year":
+        if field == "modified.year":
             return (
                 DateTimeFormatter(self.photo.date_modified).year
                 if self.photo.date_modified
                 else None
             )
 
-        if field =="modified.yy":
+        if field == "modified.yy":
             return (
-                DateTimeFormatter(self.photo.date_modified).yy if self.photo.date_modified else None
+                DateTimeFormatter(self.photo.date_modified).yy
+                if self.photo.date_modified
+                else None
             )
 
-        if field =="modified.mm":
+        if field == "modified.mm":
             return (
-                DateTimeFormatter(self.photo.date_modified).mm if self.photo.date_modified else None
+                DateTimeFormatter(self.photo.date_modified).mm
+                if self.photo.date_modified
+                else None
             )
 
-        if field =="modified.month":
+        if field == "modified.month":
             return (
                 DateTimeFormatter(self.photo.date_modified).month
                 if self.photo.date_modified
                 else None
             )
 
-        if field =="modified.mon":
+        if field == "modified.mon":
             return (
                 DateTimeFormatter(self.photo.date_modified).mon
                 if self.photo.date_modified
                 else None
             )
 
-        if field =="modified.dd":
+        if field == "modified.dd":
             return (
-                DateTimeFormatter(self.photo.date_modified).dd if self.photo.date_modified else None
+                DateTimeFormatter(self.photo.date_modified).dd
+                if self.photo.date_modified
+                else None
             )
 
-        if field =="modified.doy":
+        if field == "modified.doy":
             return (
                 DateTimeFormatter(self.photo.date_modified).doy
                 if self.photo.date_modified
                 else None
             )
 
-        if field =="place.name":
+        if field == "place.name":
             return self.photo.place.name if self.photo.place else None
 
-        if field =="place.country_code":
+        if field == "place.country_code":
             return self.photo.place.country_code if self.photo.place else None
 
-        if field =="place.name.country":
+        if field == "place.name.country":
             return (
                 self.photo.place.names.country[0]
                 if self.photo.place and self.photo.place.names.country
                 else None
             )
 
-        if field =="place.name.state_province":
+        if field == "place.name.state_province":
             return (
                 self.photo.place.names.state_province[0]
                 if self.photo.place and self.photo.place.names.state_province
                 else None
             )
 
-        if field =="place.name.city":
+        if field == "place.name.city":
             return (
                 self.photo.place.names.city[0]
                 if self.photo.place and self.photo.place.names.city
                 else None
             )
 
-        if field =="place.name.area_of_interest":
+        if field == "place.name.area_of_interest":
             return (
                 self.photo.place.names.area_of_interest[0]
                 if self.photo.place and self.photo.place.names.area_of_interest
                 else None
             )
 
-        if field =="place.address":
+        if field == "place.address":
             return (
                 self.photo.place.address_str
                 if self.photo.place and self.photo.place.address_str
                 else None
             )
 
-        if field =="place.address.street":
+        if field == "place.address.street":
             return (
                 self.photo.place.address.street
                 if self.photo.place and self.photo.place.address.street
                 else None
             )
 
-        if field =="place.address.city":
+        if field == "place.address.city":
             return (
                 self.photo.place.address.city
                 if self.photo.place and self.photo.place.address.city
                 else None
             )
 
-        if field =="place.address.state_province":
+        if field == "place.address.state_province":
             return (
                 self.photo.place.address.state_province
                 if self.photo.place and self.photo.place.address.state_province
                 else None
             )
 
-        if field =="place.address.postal_code":
+        if field == "place.address.postal_code":
             return (
                 self.photo.place.address.postal_code
                 if self.photo.place and self.photo.place.address.postal_code
                 else None
             )
 
-        if field =="place.address.country":
+        if field == "place.address.country":
             return (
                 self.photo.place.address.country
                 if self.photo.place and self.photo.place.address.country
                 else None
             )
 
-        if field =="place.address.country_code":
+        if field == "place.address.country_code":
             return (
                 self.photo.place.address.iso_country_code
                 if self.photo.place and self.photo.place.address.iso_country_code

osxphotos/placeinfo.py

@@ -87,19 +87,19 @@ class PLRevGeoLocationInfo:
         self.postalAddress = postalAddress
 
     def __eq__(self, other):
-        for field in [
-            "addressString",
-            "countryCode",
-            "isHome",
-            "compoundNames",
-            "compoundSecondaryNames",
-            "version",
-            "geoServiceProvider",
-            "postalAddress",
-        ]:
-            if getattr(self, field) != getattr(other, field):
-                return False
-        return True
+        return all(
+            getattr(self, field) == getattr(other, field)
+            for field in [
+                "addressString",
+                "countryCode",
+                "isHome",
+                "compoundNames",
+                "compoundSecondaryNames",
+                "version",
+                "geoServiceProvider",
+                "postalAddress",
+            ]
+        )
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -151,21 +151,17 @@ class PLRevGeoMapItem:
         self.finalPlaceInfos = finalPlaceInfos
 
     def __eq__(self, other):
-        for field in ["sortedPlaceInfos", "finalPlaceInfos"]:
-            if getattr(self, field) != getattr(other, field):
-                return False
-        return True
+        return all(
+            getattr(self, field) == getattr(other, field)
+            for field in ["sortedPlaceInfos", "finalPlaceInfos"]
+        )
 
     def __ne__(self, other):
         return not self.__eq__(other)
 
     def __str__(self):
-        sortedPlaceInfos = []
-        finalPlaceInfos = []
-        for place in self.sortedPlaceInfos:
-            sortedPlaceInfos.append(str(place))
-        for place in self.finalPlaceInfos:
-            finalPlaceInfos.append(str(place))
+        sortedPlaceInfos = [str(place) for place in self.sortedPlaceInfos]
+        finalPlaceInfos = [str(place) for place in self.finalPlaceInfos]
         return (
             f"finalPlaceInfos: {finalPlaceInfos}, sortedPlaceInfos: {sortedPlaceInfos}"
         )
@@ -192,10 +188,10 @@ class PLRevGeoMapItemAdditionalPlaceInfo:
         self.dominantOrderType = dominantOrderType
 
     def __eq__(self, other):
-        for field in ["area", "name", "placeType", "dominantOrderType"]:
-            if getattr(self, field) != getattr(other, field):
-                return False
-        return True
+        return all(
+            getattr(self, field) == getattr(other, field)
+            for field in ["area", "name", "placeType", "dominantOrderType"]
+        )
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -245,19 +241,19 @@ class CNPostalAddress:
         self._subLocality = _subLocality
 
     def __eq__(self, other):
-        for field in [
-            "_ISOCountryCode",
-            "_city",
-            "_country",
-            "_postalCode",
-            "_state",
-            "_street",
-            "_subAdministrativeArea",
-            "_subLocality",
-        ]:
-            if getattr(self, field) != getattr(other, field):
-                return False
-        return True
+        return all(
+            getattr(self, field) == getattr(other, field)
+            for field in [
+                "_ISOCountryCode",
+                "_city",
+                "_country",
+                "_postalCode",
+                "_state",
+                "_street",
+                "_subAdministrativeArea",
+                "_subLocality",
+            ]
+        )
 
     def __ne__(self, other):
         return not self.__eq__(other)
@@ -490,16 +486,14 @@ class PlaceInfo4(PlaceInfo):
             "names": self.names,
             "country_code": self.country_code,
         }
-        strval = "PlaceInfo(" + ", ".join([f"{k}='{v}'" for k, v in info.items()]) + ")"
-        return strval
+        return "PlaceInfo(" + ", ".join([f"{k}='{v}'" for k, v in info.items()]) + ")"
 
     def as_dict(self):
-        info = {
+        return {
             "name": self.name,
             "names": self.names._asdict(),
             "country_code": self.country_code,
         }
-        return info
 
 
 class PlaceInfo5(PlaceInfo):
@@ -541,7 +535,7 @@ class PlaceInfo5(PlaceInfo):
     @property
     def address(self):
         addr = self._plrevgeoloc.postalAddress
-        address = PostalAddress(
+        return PostalAddress(
             street=addr._street,
             sub_locality=addr._subLocality,
             city=addr._city,
@@ -551,7 +545,6 @@ class PlaceInfo5(PlaceInfo):
             country=addr._country,
             iso_country_code=addr._ISOCountryCode,
         )
-        return address
 
     def _process_place_info(self):
         """ Process sortedPlaceInfos to set self._name and self._names """
@@ -630,11 +623,10 @@ class PlaceInfo5(PlaceInfo):
             "address_str": self.address_str,
             "address": str(self.address),
         }
-        strval = "PlaceInfo(" + ", ".join([f"{k}='{v}'" for k, v in info.items()]) + ")"
-        return strval
+        return "PlaceInfo(" + ", ".join([f"{k}='{v}'" for k, v in info.items()]) + ")"
 
     def as_dict(self):
-        info = {
+        return {
             "name": self.name,
             "names": self.names._asdict(),
             "country_code": self.country_code,
@@ -642,4 +634,3 @@ class PlaceInfo5(PlaceInfo):
             "address_str": self.address_str,
             "address": self.address._asdict(),
         }
-        return info

osxphotos/utils.py

@@ -261,10 +261,9 @@ def get_preferred_uti_extension(uti):
 
     # reference: https://developer.apple.com/documentation/coreservices/1442744-uttypecopypreferredtagwithclass?language=objc
 
-    ext = CoreServices.UTTypeCopyPreferredTagWithClass(
+    return CoreServices.UTTypeCopyPreferredTagWithClass(
         uti, CoreServices.kUTTagClassFilenameExtension
     )
-    return ext
 
 
 def findfiles(pattern, path_):

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

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

tests/Test-10.15.5.photoslibrary/private/com.apple.photoanalysisd/caches/graph/PhotoAnalysisServicePreferences.plist

@@ -3,24 +3,24 @@
 <plist version="1.0">
 <dict>
 	<key>BackgroundHighlightCollection</key>
-	<date>2020-05-30T01:45:51Z</date>
+	<date>2020-06-06T14:26:31Z</date>
 	<key>BackgroundHighlightEnrichment</key>
-	<date>2020-05-30T01:45:51Z</date>
+	<date>2020-06-06T14:26:29Z</date>
 	<key>BackgroundJobAssetRevGeocode</key>
-	<date>2020-05-30T04:01:24Z</date>
+	<date>2020-06-06T14:26:31Z</date>
 	<key>BackgroundJobSearch</key>
-	<date>2020-05-30T01:45:51Z</date>
+	<date>2020-06-06T14:26:31Z</date>
 	<key>BackgroundPeopleSuggestion</key>
-	<date>2020-05-30T01:45:51Z</date>
+	<date>2020-06-06T14:26:29Z</date>
 	<key>BackgroundUserBehaviorProcessor</key>
-	<date>2020-05-29T04:31:38Z</date>
+	<date>2020-06-06T14:26:31Z</date>
 	<key>PhotoAnalysisGraphLastBackgroundGraphConsistencyUpdateJobDateKey</key>
 	<date>2020-05-30T02:16:06Z</date>
 	<key>PhotoAnalysisGraphLastBackgroundGraphRebuildJobDate</key>
 	<date>2020-05-29T04:31:37Z</date>
 	<key>PhotoAnalysisGraphLastBackgroundMemoryGenerationJobDate</key>
-	<date>2020-05-30T04:01:24Z</date>
+	<date>2020-06-06T14:26:33Z</date>
 	<key>SiriPortraitDonation</key>
-	<date>2020-05-29T04:31:38Z</date>
+	<date>2020-06-06T14:26:31Z</date>
 </dict>
 </plist>

tests/test_catalina_10_15_5.py

@@ -7,6 +7,8 @@ PHOTOS_DB = "tests/Test-10.15.5.photoslibrary/database/photos.db"
 PHOTOS_DB_PATH = "/Test-10.15.5.photoslibrary/database/photos.db"
 PHOTOS_LIBRARY_PATH = "/Test-10.15.5.photoslibrary"
 
+PHOTOS_DB_LEN = 13
+
 KEYWORDS = [
     "Kids",
     "wedding",
@@ -22,10 +24,11 @@ KEYWORDS = [
 PERSONS = ["Katie", "Suzy", "Maria", _UNKNOWN_PERSON]
 ALBUMS = [
     "Pumpkin Farm",
-    "Test Album",
+    "Test Album",  # there are 2 albums named "Test Album" for testing duplicate album names
     "AlbumInFolder",
-    "Raw"
-]  # Note: there are 2 albums named "Test Album" for testing duplicate album names
+    "Raw",
+    "I have a deleted twin",  # there's an empty album with same name that has been deleted
+]
 KEYWORDS_DICT = {
     "Kids": 4,
     "wedding": 2,
@@ -43,6 +46,7 @@ ALBUM_DICT = {
     "Test Album": 2,
     "AlbumInFolder": 2,
     "Raw": 4,
+    "I have a deleted twin": 1,
 }  # Note: there are 2 albums named "Test Album" for testing duplicate album names
 
 UUID_DICT = {
@@ -58,6 +62,7 @@ UUID_DICT = {
     "external_edit": "DC99FBDD-7A52-4100-A5BB-344131646C30",
     "no_external_edit": "E9BC5C36-7CD1-40A1-A72B-8B8FAC227D51",
     "export": "D79B8D77-BFFC-460B-9312-034F2877D35B",  # "Pumkins2.jpg"
+    "export_tif": "8846E3E6-8AC8-4857-8448-E3D025784410",
 }
 
 
@@ -109,14 +114,14 @@ def test_init4():
 def test_init5(mocker):
     # test failed get_last_library_path
     import osxphotos
-    
+
     def bad_library():
         return None
 
     # get_last_library actually in utils but need to patch it in photosdb because it's imported into photosdb
     # because of the layout of photosdb/ need to patch it this way...don't really understand why, but it works
     mocker.patch("osxphotos.photosdb.photosdb.get_last_library_path", new=bad_library)
-    
+
     with pytest.raises(Exception):
         assert osxphotos.PhotosDB()
 
@@ -126,7 +131,7 @@ def test_db_len():
 
     photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
     # assert photosdb.db_version in osxphotos._TESTED_DB_VERSIONS
-    assert len(photosdb) == 12 
+    assert len(photosdb) == PHOTOS_DB_LEN
 
 
 def test_db_version():
@@ -229,7 +234,7 @@ def test_missing():
     photos = photosdb.photos(uuid=[UUID_DICT["missing"]])
     assert len(photos) == 1
     p = photos[0]
-    assert p.path == None
+    assert p.path is None
     assert p.ismissing == True
 
 
@@ -378,7 +383,7 @@ def test_count():
 
     photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
     photos = photosdb.photos()
-    assert len(photos) == 12 
+    assert len(photos) == PHOTOS_DB_LEN
 
 
 def test_keyword_2():
@@ -730,6 +735,31 @@ def test_export_13():
     assert e.type == type(FileNotFoundError())
 
 
+def test_export_14(caplog):
+    # test export with user provided filename with different (but valid) extension than source
+    import os
+    import os.path
+    import tempfile
+    import time
+
+    import osxphotos
+
+    tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_")
+    dest = tempdir.name
+    photosdb = osxphotos.PhotosDB(dbfile=PHOTOS_DB)
+    photos = photosdb.photos(uuid=[UUID_DICT["export_tif"]])
+
+    timestamp = time.time()
+    filename = f"osxphotos-export-2-test-{timestamp}.tif"
+    expected_dest = os.path.join(dest, filename)
+    got_dest = photos[0].export(dest, filename)[0]
+
+    assert got_dest == expected_dest
+    assert os.path.isfile(got_dest)
+
+    assert "Invalid destination suffix" not in caplog.text
+
+
 def test_eq():
     import osxphotos
 
@@ -781,7 +811,7 @@ def test_from_to_date():
     photosdb = osxphotos.PhotosDB(PHOTOS_DB)
 
     photos = photosdb.photos(from_date=dt.datetime(2018, 10, 28))
-    assert len(photos) ==6 
+    assert len(photos) == 7
 
     photos = photosdb.photos(to_date=dt.datetime(2018, 10, 28))
     assert len(photos) == 6

tests/test_cli.py

@@ -49,6 +49,10 @@ CLI_EXPORT_FILENAMES = [
     "wedding_edited.jpeg",
 ]
 
+CLI_EXPORT_FILENAMES_ALBUM = ["Pumkins1.jpg", "Pumkins2.jpg", "Pumpkins3.jpg"]
+
+CLI_EXPORT_FILENAMES_DELETED_TWIN = ["wedding.jpg", "wedding_edited.jpeg"]
+
 CLI_EXPORT_EDITED_SUFFIX = "_bearbeiten"
 
 CLI_EXPORT_FILENAMES_EDITED_SUFFIX = [
@@ -164,6 +168,9 @@ CLI_EXPORTED_FILENAME_TEMPLATE_FILENAMES2 = [
     "Pumpkin Farm-Pumpkins3.jpg",
     "Test Album-Pumkins1.jpg",
     "Test Album-Pumkins2.jpg",
+    "None-IMG_1693.tif",
+    "I have a deleted twin-wedding.jpg",
+    "I have a deleted twin-wedding_edited.jpeg",
 ]
 
 CLI_EXPORT_UUID = "D79B8D77-BFFC-460B-9312-034F2877D35B"
@@ -946,6 +953,54 @@ def test_export_filename_template_3():
         assert "Error: Invalid template" in result.output
 
 
+def test_export_album():
+    """Test export of an album """
+    import glob
+    import os
+    import os.path
+    import osxphotos
+    from osxphotos.__main__ import export
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            export,
+            [os.path.join(cwd, PHOTOS_DB_15_5), ".", "--album", "Pumpkin Farm", "-V"],
+        )
+        assert result.exit_code == 0
+        files = glob.glob("*")
+        assert sorted(files) == sorted(CLI_EXPORT_FILENAMES_ALBUM)
+
+
+def test_export_album_deleted_twin():
+    """Test export of an album where album of same name has been deleted """
+    import glob
+    import os
+    import os.path
+    import osxphotos
+    from osxphotos.__main__ import export
+
+    runner = CliRunner()
+    cwd = os.getcwd()
+    # pylint: disable=not-context-manager
+    with runner.isolated_filesystem():
+        result = runner.invoke(
+            export,
+            [
+                os.path.join(cwd, PHOTOS_DB_15_5),
+                ".",
+                "--album",
+                "I have a deleted twin",
+                "-V",
+            ],
+        )
+        assert result.exit_code == 0
+        files = glob.glob("*")
+        assert sorted(files) == sorted(CLI_EXPORT_FILENAMES_DELETED_TWIN)
+
+
 def test_places():
     import json
     import os