diff --git a/osxphotos/_version.py b/osxphotos/_version.py index 5c53d9e1..cd54e994 100644 --- a/osxphotos/_version.py +++ b/osxphotos/_version.py @@ -1,4 +1,4 @@ """ version info """ -__version__ = "0.36.7" +__version__ = "0.36.8" diff --git a/osxphotos/exiftool.py b/osxphotos/exiftool.py index 3cdb27af..2c36343e 100644 --- a/osxphotos/exiftool.py +++ b/osxphotos/exiftool.py @@ -100,7 +100,7 @@ class _ExifToolProc: ], stdin=subprocess.PIPE, stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, + stderr=subprocess.STDOUT, ) self._process_running = True @@ -133,13 +133,19 @@ class ExifTool: """ Basic exiftool interface for reading and writing EXIF tags """ def __init__(self, filepath, exiftool=None, overwrite=True): - """ Return ExifTool object - file: path to image file - exiftool: path to exiftool, if not specified will look in path - overwrite: if True, will overwrite image file without creating backup, default=False """ + """ Create ExifTool object + + Args: + file: path to image file + exiftool: path to exiftool, if not specified will look in path + overwrite: if True, will overwrite image file without creating backup, default=False + Returns: + ExifTool instance + """ self.file = filepath self.overwrite = overwrite self.data = {} + self.error = None # if running as a context manager, self._context_mgr will be True self._context_mgr = False self._exiftoolproc = _ExifToolProc(exiftool=exiftool) @@ -147,8 +153,18 @@ class ExifTool: self._read_exif() 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: + tag: str; name of tag to set + value: str; value to set tag to + + Returns: + True if success otherwise False + + If error generated by exiftool, returns False and sets self.error to error string + If called in context manager, returns True (execution is delayed until exiting context manager) + """ if value is None: value = "" @@ -157,19 +173,32 @@ class ExifTool: command.append("-overwrite_original") if self._context_mgr: self._commands.extend(command) + return True else: - self.run_commands(*command) + _, self.error = self.run_commands(*command) + return self.error is None def addvalues(self, tag, *values): """ Add one or more value(s) to tag If more than one value is passed, each value will be added to the tag - 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 - 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. - 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, - but if addvalues called without first calling setvalue, exiftool will add duplicate values + + Args: + tag: str; tag to set + *values: str; one or more values to set + + Returns: + True if success otherwise False + + If error generated by exiftool, returns False and sets self.error to error string + If called in context manager, returns True (execution is delayed until exiting context manager) + + Notes: exiftool may add duplicate values for some tags so the caller must ensure + 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, + 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 + 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 """ if not values: raise ValueError("Must pass at least one value") @@ -185,14 +214,26 @@ class ExifTool: if self._context_mgr: self._commands.extend(command) + return True else: - self.run_commands(*command) + _, self.error = self.run_commands(*command) + return self.error is None def run_commands(self, *commands, no_file=False): - """ run commands in the exiftool process and return result - 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 """ + """ Run commands in the exiftool process and return result. + + Args: + *commands: exiftool commands to run + 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 + Returns: + (output, errror) + output: bytes is containing output of exiftool commands + error: if exiftool generated an error, bytes containing error string otherwise None + + Note: Also sets self.error if error generated. + """ if not (hasattr(self, "_process") and self._process): raise ValueError("exiftool process is not running") @@ -218,9 +259,16 @@ class ExifTool: # read the output output = b"" + error = b"" while EXIFTOOL_STAYOPEN_EOF not in str(output): - output += self._process.stdout.readline().strip() - return output[:-EXIFTOOL_STAYOPEN_EOF_LEN] + line = self._process.stdout.readline() + if line.startswith(b"Warning"): + error += line + else: + output += line.strip() + error = None if error == b"" else error + self.error = error + return output[:-EXIFTOOL_STAYOPEN_EOF_LEN], error @property def pid(self): @@ -230,14 +278,14 @@ class ExifTool: @property def version(self): """ returns exiftool version """ - ver = self.run_commands("-ver", no_file=True) + ver, _ = self.run_commands("-ver", no_file=True) return ver.decode("utf-8") def asdict(self): """ return dictionary of all EXIF tags and values from exiftool returns empty dict if no tags """ - json_str = self.run_commands("-json") + json_str, _ = self.run_commands("-json") if json_str: exifdict = json.loads(json_str) return exifdict[0] @@ -246,7 +294,8 @@ class ExifTool: def json(self): """ returns JSON string containing all EXIF tags and values from exiftool """ - return self.run_commands("-json") + json, _ = self.run_commands("-json") + return json def _read_exif(self): """ read exif data from file """ @@ -265,4 +314,4 @@ class ExifTool: if exc_type: return False elif self._commands: - self.run_commands(*self._commands) + _, self.error = self.run_commands(*self._commands) diff --git a/tests/test_exiftool.py b/tests/test_exiftool.py index 073458a7..09ecf02c 100644 --- a/tests/test_exiftool.py +++ b/tests/test_exiftool.py @@ -103,10 +103,28 @@ def test_setvalue_1(): exif = osxphotos.exiftool.ExifTool(tempfile) exif.setvalue("IPTC:Keywords", "test") + assert not exif.error + exif._read_exif() assert exif.data["IPTC:Keywords"] == "test" +def test_setvalue_error(): + # test setting illegal tag value generates error + import os.path + import tempfile + import osxphotos.exiftool + from osxphotos.fileutil import FileUtil + + tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_") + tempfile = os.path.join(tempdir.name, os.path.basename(TEST_FILE_ONE_KEYWORD)) + FileUtil.copy(TEST_FILE_ONE_KEYWORD, tempfile) + + exif = osxphotos.exiftool.ExifTool(tempfile) + exif.setvalue("IPTC:Foo", "test") + assert exif.error + + def test_setvalue_context_manager(): # test setting a tag value as context manager import os.path @@ -124,6 +142,8 @@ def test_setvalue_context_manager(): exif.setvalue("XMP:Title", "title") exif.setvalue("XMP:Subject", "subject") + assert exif.error is None + exif2 = osxphotos.exiftool.ExifTool(tempfile) exif2._read_exif() assert sorted(exif2.data["IPTC:Keywords"]) == ["test1", "test2"] @@ -131,6 +151,22 @@ def test_setvalue_context_manager(): assert exif2.data["XMP:Subject"] == "subject" +def test_setvalue_context_manager_error(): + # test setting a tag value as context manager when error generated + import os.path + import tempfile + import osxphotos.exiftool + from osxphotos.fileutil import FileUtil + + tempdir = tempfile.TemporaryDirectory(prefix="osxphotos_") + tempfile = os.path.join(tempdir.name, os.path.basename(TEST_FILE_ONE_KEYWORD)) + FileUtil.copy(TEST_FILE_ONE_KEYWORD, tempfile) + + with osxphotos.exiftool.ExifTool(tempfile) as exif: + exif.setvalue("Foo:Bar", "test1") + assert exif.error + + def test_clear_value(): # test clearing a tag value import os.path diff --git a/tests/test_export_catalina_10_15_7.py b/tests/test_export_catalina_10_15_7.py index 8de901e6..c60a8440 100644 --- a/tests/test_export_catalina_10_15_7.py +++ b/tests/test_export_catalina_10_15_7.py @@ -744,7 +744,7 @@ def test_xmp_sidecar_is_valid(tmp_path): xmp_file = tmp_path / XMP_FILENAME assert xmp_file.is_file() exiftool = ExifTool(str(xmp_file)) - output = exiftool.run_commands("-validate", "-warning") + output, _ = exiftool.run_commands("-validate", "-warning") assert output == b"[ExifTool] Validate : 0 0 0"