Fixed --exiftool-path bug, issue #311, #313

This commit is contained in:
Rhet Turnbull
2020-12-30 20:21:05 -08:00
parent 27282af3b9
commit 3394c52768
6 changed files with 124 additions and 76 deletions

View File

@@ -2420,9 +2420,9 @@ def _query(
has_likes=False, has_likes=False,
no_likes=False, no_likes=False,
): ):
""" Run a query against PhotosDB to extract the photos based on user supply criteria used by query and export commands """Run a query against PhotosDB to extract the photos based on user supply criteria used by query and export commands
Args: Args:
photosdb: PhotosDB object photosdb: PhotosDB object
""" """
@@ -3201,8 +3201,8 @@ def load_uuid_from_file(filename):
def write_export_report(report_file, results): def write_export_report(report_file, results):
""" write CSV report with results from export """write CSV report with results from export
Args: Args:
report_file: path to report file report_file: path to report file
results: ExportResults object results: ExportResults object
@@ -3331,7 +3331,7 @@ def write_export_report(report_file, results):
def cleanup_files(dest_path, files_to_keep, fileutil): def cleanup_files(dest_path, files_to_keep, fileutil):
""" cleanup dest_path by deleting and files and empty directories """cleanup dest_path by deleting and files and empty directories
not in files_to_keep not in files_to_keep
Args: Args:
@@ -3375,8 +3375,8 @@ def write_finder_tags(
exiftool_merge_keywords=None, exiftool_merge_keywords=None,
finder_tag_template=None, finder_tag_template=None,
): ):
""" Write Finder tags (extended attributes) to files; only writes attributes if attributes on file differ from what would be written """Write Finder tags (extended attributes) to files; only writes attributes if attributes on file differ from what would be written
Args: Args:
photo: a PhotoInfo object photo: a PhotoInfo object
files: list of file paths to write Finder tags to files: list of file paths to write Finder tags to
@@ -3384,7 +3384,7 @@ def write_finder_tags(
keyword_template: list of keyword templates to evaluate for determining keywords keyword_template: list of keyword templates to evaluate for determining keywords
album_keyword: if True, use album names as keywords album_keyword: if True, use album names as keywords
person_keyword: if True, use person in image as keywords person_keyword: if True, use person in image as keywords
exiftool_merge_keywords: if True, include any keywords in the exif data of the source image as keywords exiftool_merge_keywords: if True, include any keywords in the exif data of the source image as keywords
finder_tag_template: list of templates to evaluate for determining Finder tags finder_tag_template: list of templates to evaluate for determining Finder tags
Returns: Returns:

View File

@@ -1,5 +1,5 @@
""" version info """ """ version info """
__version__ = "0.39.0" __version__ = "0.39.1"

View File

@@ -33,8 +33,8 @@ def get_exiftool_path():
class _ExifToolProc: class _ExifToolProc:
""" Runs exiftool in a subprocess via Popen """Runs exiftool in a subprocess via Popen
Creates a singleton object """ Creates a singleton object"""
def __new__(cls, *args, **kwargs): def __new__(cls, *args, **kwargs):
""" create new object or return instance of already created singleton """ """ create new object or return instance of already created singleton """
@@ -44,8 +44,8 @@ class _ExifToolProc:
return cls.instance return cls.instance
def __init__(self, exiftool=None): def __init__(self, exiftool=None):
""" construct _ExifToolProc singleton object or return instance of already created object """construct _ExifToolProc singleton object or return instance of already created object
exiftool: optional path to exiftool binary (if not provided, will search path to find it) """ exiftool: optional path to exiftool binary (if not provided, will search path to find it)"""
if hasattr(self, "_process_running") and self._process_running: if hasattr(self, "_process_running") and self._process_running:
# already running # already running
@@ -56,8 +56,8 @@ class _ExifToolProc:
) )
return return
self._exiftool = exiftool or get_exiftool_path()
self._process_running = False self._process_running = False
self._exiftool = exiftool or get_exiftool_path()
self._start_proc() self._start_proc()
@property @property
@@ -106,8 +106,8 @@ class _ExifToolProc:
def _stop_proc(self): def _stop_proc(self):
""" stop the exiftool process if it's running, otherwise, do nothing """ """ stop the exiftool process if it's running, otherwise, do nothing """
if not self._process_running: if not self._process_running:
logging.warning("exiftool process is not running")
return return
self._process.stdin.write(b"-stay_open\n") self._process.stdin.write(b"-stay_open\n")
@@ -133,7 +133,7 @@ class ExifTool:
""" Basic exiftool interface for reading and writing EXIF tags """ """ Basic exiftool interface for reading and writing EXIF tags """
def __init__(self, filepath, exiftool=None, overwrite=True, flags=None): def __init__(self, filepath, exiftool=None, overwrite=True, flags=None):
""" Create ExifTool object """Create ExifTool object
Args: Args:
file: path to image file file: path to image file
@@ -157,15 +157,15 @@ class ExifTool:
self._read_exif() self._read_exif()
def setvalue(self, tag, value): def setvalue(self, tag, value):
""" Set tag to value(s); if value is None, will delete tag """Set tag to value(s); if value is None, will delete tag
Args: Args:
tag: str; name of tag to set tag: str; name of tag to set
value: str; value to set tag to value: str; value to set tag to
Returns: Returns:
True if success otherwise False True if success otherwise False
If error generated by exiftool, returns False and sets self.error to error string 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 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) If called in context manager, returns True (execution is delayed until exiting context manager)
@@ -184,26 +184,26 @@ class ExifTool:
return error is None return error is None
def addvalues(self, tag, *values): def addvalues(self, tag, *values):
""" Add one or more value(s) to tag """Add one or more value(s) to tag
If more than one value is passed, each value will be added to the tag If more than one value is passed, each value will be added to the tag
Args: Args:
tag: str; tag to set tag: str; tag to set
*values: str; one or more values to set *values: str; one or more values to set
Returns: Returns:
True if success otherwise False True if success otherwise False
If error generated by exiftool, returns False and sets self.error to error string 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 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) 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 Notes: exiftool may add duplicate values for some tags so the caller must ensure
the values being added are not already in the EXIF data the values being added are not already in the EXIF data
For some tags, such as IPTC:Keywords, this will add a new value to the list of keywords, For some tags, such as IPTC:Keywords, this will add a new value to the list of keywords,
but for others, such as EXIF:ISO, this will literally add a value to the existing value. but for others, such as EXIF:ISO, this will literally add a value to the existing value.
It's up to the caller to know what exiftool will do for each tag It's up to the caller to know what exiftool will do for each tag
If setvalue called before addvalues, exiftool does not appear to add duplicates, If setvalue called before addvalues, exiftool does not appear to add duplicates,
but if addvalues called without first calling setvalue, exiftool will add duplicate values but if addvalues called without first calling setvalue, exiftool will add duplicate values
""" """
if not values: if not values:
@@ -226,7 +226,7 @@ class ExifTool:
return error is None return error is None
def run_commands(self, *commands, no_file=False): def run_commands(self, *commands, no_file=False):
""" Run commands in the exiftool process and return result. """Run commands in the exiftool process and return result.
Args: Args:
*commands: exiftool commands to run *commands: exiftool commands to run
@@ -301,8 +301,8 @@ class ExifTool:
return ver.decode("utf-8") return ver.decode("utf-8")
def asdict(self): def asdict(self):
""" return dictionary of all EXIF tags and values from exiftool """return dictionary of all EXIF tags and values from exiftool
returns empty dict if no tags returns empty dict if no tags
""" """
json_str, _, _ = self.run_commands("-json") json_str, _, _ = self.run_commands("-json")
if json_str: if json_str:

View File

@@ -86,8 +86,8 @@ class PhotoInfo:
@property @property
def original_filename(self): def original_filename(self):
""" original filename of the picture """original filename of the picture
Photos 5 mangles filenames upon import """ Photos 5 mangles filenames upon import"""
if ( if (
self._db._db_version <= _PHOTOS_4_VERSION self._db._db_version <= _PHOTOS_4_VERSION
and self.has_raw and self.has_raw
@@ -106,8 +106,8 @@ class PhotoInfo:
@property @property
def date_modified(self): def date_modified(self):
""" image modification date as timezone aware datetime object """image modification date as timezone aware datetime object
or None if no modification date set """ or None if no modification date set"""
# Photos <= 4 provides no way to get date of adjustment and will update # Photos <= 4 provides no way to get date of adjustment and will update
# lastmodifieddate anytime photo database record is updated (e.g. adding tags) # lastmodifieddate anytime photo database record is updated (e.g. adding tags)
@@ -492,9 +492,9 @@ class PhotoInfo:
@property @property
def ismissing(self): def ismissing(self):
""" returns true if photo is missing from disk (which means it's not been downloaded from iCloud) """returns true if photo is missing from disk (which means it's not been downloaded from iCloud)
NOTE: the photos.db database uses an asynchrounous write-ahead log so changes in Photos NOTE: the photos.db database uses an asynchrounous write-ahead log so changes in Photos
do not immediately get written to disk. In particular, I've noticed that downloading do not immediately get written to disk. In particular, I've noticed that downloading
an image from the cloud does not force the database to be updated until something else an image from the cloud does not force the database to be updated until something else
e.g. an edit, keyword, etc. occurs forcing a database synch e.g. an edit, keyword, etc. occurs forcing a database synch
The exact process / timing is a mystery to be but be aware that if some photos were recently The exact process / timing is a mystery to be but be aware that if some photos were recently
@@ -539,8 +539,8 @@ class PhotoInfo:
@property @property
def shared(self): def shared(self):
""" returns True if photos is in a shared iCloud album otherwise false """returns True if photos is in a shared iCloud album otherwise false
Only valid on Photos 5; returns None on older versions """ Only valid on Photos 5; returns None on older versions"""
if self._db._db_version > _PHOTOS_4_VERSION: if self._db._db_version > _PHOTOS_4_VERSION:
return self._info["shared"] return self._info["shared"]
else: else:
@@ -548,8 +548,8 @@ class PhotoInfo:
@property @property
def uti(self): def uti(self):
""" Returns Uniform Type Identifier (UTI) for the image """Returns Uniform Type Identifier (UTI) for the image
for example: public.jpeg or com.apple.quicktime-movie for example: public.jpeg or com.apple.quicktime-movie
""" """
if self._db._db_version <= _PHOTOS_4_VERSION: if self._db._db_version <= _PHOTOS_4_VERSION:
if self.hasadjustments: if self.hasadjustments:
@@ -564,8 +564,8 @@ class PhotoInfo:
@property @property
def uti_original(self): def uti_original(self):
""" Returns Uniform Type Identifier (UTI) for the original image """Returns Uniform Type Identifier (UTI) for the original image
for example: public.jpeg or com.apple.quicktime-movie for example: public.jpeg or com.apple.quicktime-movie
""" """
if self._db._db_version <= _PHOTOS_4_VERSION and self._info["has_raw"]: if self._db._db_version <= _PHOTOS_4_VERSION and self._info["has_raw"]:
return self._info["raw_pair_info"]["UTI"] return self._info["raw_pair_info"]["UTI"]
@@ -577,9 +577,9 @@ class PhotoInfo:
@property @property
def uti_edited(self): def uti_edited(self):
""" Returns Uniform Type Identifier (UTI) for the edited image """Returns Uniform Type Identifier (UTI) for the edited image
if the photo has been edited, otherwise None; if the photo has been edited, otherwise None;
for example: public.jpeg for example: public.jpeg
""" """
if self._db._db_version >= _PHOTOS_5_VERSION: if self._db._db_version >= _PHOTOS_5_VERSION:
return self.uti if self.hasadjustments else None return self.uti if self.hasadjustments else None
@@ -588,36 +588,34 @@ class PhotoInfo:
@property @property
def uti_raw(self): def uti_raw(self):
""" Returns Uniform Type Identifier (UTI) for the RAW image if there is one """Returns Uniform Type Identifier (UTI) for the RAW image if there is one
for example: com.canon.cr2-raw-image for example: com.canon.cr2-raw-image
Returns None if no associated RAW image Returns None if no associated RAW image
""" """
return self._info["UTI_raw"] return self._info["UTI_raw"]
@property @property
def ismovie(self): def ismovie(self):
""" Returns True if file is a movie, otherwise False """Returns True if file is a movie, otherwise False"""
"""
return True if self._info["type"] == _MOVIE_TYPE else False return True if self._info["type"] == _MOVIE_TYPE else False
@property @property
def isphoto(self): def isphoto(self):
""" Returns True if file is an image, otherwise False """Returns True if file is an image, otherwise False"""
"""
return True if self._info["type"] == _PHOTO_TYPE else False return True if self._info["type"] == _PHOTO_TYPE else False
@property @property
def incloud(self): def incloud(self):
""" Returns True if photo is cloud asset and is synched to cloud """Returns True if photo is cloud asset and is synched to cloud
False if photo is cloud asset and not yet synched to cloud False if photo is cloud asset and not yet synched to cloud
None if photo is not cloud asset None if photo is not cloud asset
""" """
return self._info["incloud"] return self._info["incloud"]
@property @property
def iscloudasset(self): def iscloudasset(self):
""" Returns True if photo is a cloud asset (in an iCloud library), """Returns True if photo is a cloud asset (in an iCloud library),
otherwise False otherwise False
""" """
if self._db._db_version <= _PHOTOS_4_VERSION: if self._db._db_version <= _PHOTOS_4_VERSION:
return ( return (
@@ -636,9 +634,9 @@ class PhotoInfo:
@property @property
def burst_photos(self): def burst_photos(self):
""" If photo is a burst photo, returns list of PhotoInfo objects """If photo is a burst photo, returns list of PhotoInfo objects
that are part of the same burst photo set; otherwise returns empty list. that are part of the same burst photo set; otherwise returns empty list.
self is not included in the returned list """ self is not included in the returned list"""
if self._info["burst"]: if self._info["burst"]:
burst_uuid = self._info["burstUUID"] burst_uuid = self._info["burstUUID"]
return [ return [
@@ -656,9 +654,9 @@ class PhotoInfo:
@property @property
def path_live_photo(self): def path_live_photo(self):
""" Returns path to the associated video file for a live photo """Returns path to the associated video file for a live photo
If photo is not a live photo, returns None If photo is not a live photo, returns None
If photo is missing, returns None """ If photo is missing, returns None"""
photopath = None photopath = None
if self._db._db_version <= _PHOTOS_4_VERSION: if self._db._db_version <= _PHOTOS_4_VERSION:
@@ -785,9 +783,9 @@ class PhotoInfo:
@property @property
def raw_original(self): def raw_original(self):
""" returns True if associated raw image and the raw image is selected in Photos """returns True if associated raw image and the raw image is selected in Photos
via "Use RAW as Original " via "Use RAW as Original "
otherwise returns False """ otherwise returns False"""
return self._info["raw_is_original"] return self._info["raw_is_original"]
@property @property
@@ -841,20 +839,20 @@ class PhotoInfo:
Args: Args:
template_str: a template string with fields to render template_str: a template string with fields to render
none_str: a str to use if template field renders to None, default is "_". none_str: a str to use if template field renders to None, default is "_".
path_sep: a single character str to use as path separator when joining path_sep: a single character str to use as path separator when joining
fields like folder_album; if not provided, defaults to os.path.sep fields like folder_album; if not provided, defaults to os.path.sep
expand_inplace: expand multi-valued substitutions in-place as a single string expand_inplace: expand multi-valued substitutions in-place as a single string
instead of returning individual strings instead of returning individual strings
inplace_sep: optional string to use as separator between multi-valued keywords inplace_sep: optional string to use as separator between multi-valued keywords
with expand_inplace; default is ',' with expand_inplace; default is ','
filename: if True, template output will be sanitized to produce valid file name filename: if True, template output will be sanitized to produce valid file name
dirname: if True, template output will be sanitized to produce valid directory name dirname: if True, template output will be sanitized to produce valid directory name
replacement: str, value to replace any illegal file path characters with; default = ":" replacement: str, value to replace any illegal file path characters with; default = ":"
Returns: Returns:
([rendered_strings], [unmatched]): tuple of list of rendered strings and list of unmatched template values ([rendered_strings], [unmatched]): tuple of list of rendered strings and list of unmatched template values
""" """
template = PhotoTemplate(self) template = PhotoTemplate(self, exiftool_path=self._db._exiftool_path)
return template.render( return template.render(
template_str, template_str,
none_str=none_str, none_str=none_str,
@@ -877,11 +875,11 @@ class PhotoInfo:
return self._info["latitude"] return self._info["latitude"]
def _get_album_uuids(self): def _get_album_uuids(self):
""" Return list of album UUIDs this photo is found in """Return list of album UUIDs this photo is found in
Filters out albums in the trash and any special album types Filters out albums in the trash and any special album types
Returns: list of album UUIDs Returns: list of album UUIDs
""" """
if self._db._db_version <= _PHOTOS_4_VERSION: if self._db._db_version <= _PHOTOS_4_VERSION:
version4 = True version4 = True

View File

@@ -149,13 +149,15 @@ MULTI_VALUE_SUBSTITUTIONS = [
class PhotoTemplate: class PhotoTemplate:
""" PhotoTemplate class to render a template string from a PhotoInfo object """ """ PhotoTemplate class to render a template string from a PhotoInfo object """
def __init__(self, photo): def __init__(self, photo, exiftool_path=None):
""" Inits PhotoTemplate class with photo, non_str, and path_sep """ Inits PhotoTemplate class with photo
Args: Args:
photo: a PhotoInfo instance. photo: a PhotoInfo instance.
exiftool_path: optional path to exiftool for use with {exiftool:} template; if not provided, will look for exiftool in $PATH
""" """
self.photo = photo self.photo = photo
self.exiftool_path = exiftool_path
# holds value of current date/time for {today.x} fields # holds value of current date/time for {today.x} fields
# gets initialized in get_template_value # gets initialized in get_template_value
@@ -517,7 +519,7 @@ class PhotoTemplate:
if not self.photo.path: if not self.photo.path:
values = [None] values = [None]
else: else:
exif = ExifTool(self.photo.path) exif = ExifTool(self.photo.path, exiftool=self.exiftool_path)
exifdict = exif.asdict() exifdict = exif.asdict()
exifdict = {k.lower(): v for (k, v) in exifdict.items()} exifdict = {k.lower(): v for (k, v) in exifdict.items()}
subfield = subfield.lower() subfield = subfield.lower()

View File

@@ -357,6 +357,7 @@ CLI_EXIFTOOL = {
"XMP:TagsList": "Kids", "XMP:TagsList": "Kids",
"XMP:Title": "I found one!", "XMP:Title": "I found one!",
"EXIF:ImageDescription": "Girl holding pumpkin", "EXIF:ImageDescription": "Girl holding pumpkin",
"EXIF:Make": "Canon",
"XMP:Description": "Girl holding pumpkin", "XMP:Description": "Girl holding pumpkin",
"XMP:PersonInImage": "Katie", "XMP:PersonInImage": "Katie",
"XMP:Subject": "Kids", "XMP:Subject": "Kids",
@@ -1114,6 +1115,54 @@ def test_export_exiftool_path():
assert exif[key] == CLI_EXIFTOOL[uuid][key] assert exif[key] == CLI_EXIFTOOL[uuid][key]
@pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
def test_export_exiftool_path_render_template():
""" test --exiftool-path with {exiftool:} template rendering """
import glob
import os
import os.path
import re
import shutil
import sys
import tempfile
from osxphotos.__main__ import export
from osxphotos.exiftool import ExifTool
from osxphotos.utils import noop
exiftool_source = osxphotos.exiftool.get_exiftool_path()
# monkey patch get_exiftool_path so it returns None
get_exiftool_path = osxphotos.exiftool.get_exiftool_path
osxphotos.exiftool.get_exiftool_path = noop
runner = CliRunner()
cwd = os.getcwd()
# pylint: disable=not-context-manager
with runner.isolated_filesystem():
tempdir = tempfile.TemporaryDirectory()
exiftool_path = os.path.join(tempdir.name, "myexiftool")
shutil.copy2(exiftool_source, exiftool_path)
for uuid in CLI_EXIFTOOL:
result = runner.invoke(
export,
[
os.path.join(cwd, PHOTOS_DB_15_6),
".",
"-V",
"--filename",
"{original_name}_{exiftool:EXIF:Make}",
"--uuid",
f"{uuid}",
"--exiftool-path",
exiftool_path,
],
)
assert result.exit_code == 0
assert re.search(r"Exporting.*Canon", result.output)
osxphotos.exiftool.get_exiftool_path = get_exiftool_path
@pytest.mark.skipif(exiftool is None, reason="exiftool not installed") @pytest.mark.skipif(exiftool is None, reason="exiftool not installed")
def test_export_exiftool_ignore_date_modified(): def test_export_exiftool_ignore_date_modified():
import glob import glob
@@ -5066,4 +5115,3 @@ def test_export_finder_tag_template_keywords():
persons = [persons] if type(persons) != list else persons persons = [persons] if type(persons) != list else persons
expected = [Tag(x) for x in keywords + persons] expected = [Tag(x) for x in keywords + persons]
assert sorted(md.tags) == sorted(expected) assert sorted(md.tags) == sorted(expected)