Refactoring of save-config/load-config code

osxphotos/__main__.py

@@ -25,7 +25,11 @@ from ._constants import (
     UNICODE_FORMAT,
 )
 from ._version import __version__
-from .configoptions import ExportOptions, InvalidOptions
+from .configoptions import (
+    ConfigOptions,
+    ConfigOptionsInvalidError,
+    ConfigOptionsLoadError,
+)
 from .datetime_formatter import DateTimeFormatter
 from .exiftool import get_exiftool_path
 from .export_db import ExportDB, ExportDBInMemory
@@ -1572,12 +1576,15 @@ def export(
     See --skip-edited, --skip-live, --skip-bursts, and --skip-raw options
     to modify this behavior.
     """
- 
+
-    # NOTE: because of the way ExportOptions works, Click options must not 
+    # NOTE: because of the way ConfigOptions works, Click options must not
     # set defaults which are not None or False. If defaults need to be set
     # do so below after load_config and save_config are handled.
- 
+    cfg = ConfigOptions(
-    cfg = ExportOptions(**locals())
+        "export",
+        locals(),
+        ignore=["ctx", "cli_obj", "dest", "load_config", "save_config"],
+    )
 
     # print(jpeg_quality, edited_suffix, original_suffix)
 
@@ -1585,11 +1592,14 @@ def export(
     VERBOSE = bool(verbose)
 
     if load_config:
-        cfg, error = ExportOptions().load_from_file(load_config, cfg)
+        try:
-        # print(cfg.asdict())
+            cfg.load_from_file(load_config)
-        if error:
+        except ConfigOptionsLoadError as e:
-            click.echo(f"Error parsing {load_config} config file: {error}", err=True)
+            click.echo(
+                f"Error parsing {load_config} config file: {e.message}", err=True
+            )
             raise click.Abort()
+
         # re-set the local function vars to the corresponding config value
         # this isn't elegant but avoids having to rewrite this function to use cfg.varname for every parameter
         db = cfg.db
@@ -1679,15 +1689,48 @@ def export(
         use_photokit = cfg.use_photokit
         report = cfg.report
         cleanup = cfg.cleanup
-        
+
         # config file might have changed verbose
         VERBOSE = bool(verbose)
         verbose_(f"Loaded options from file {load_config}")
 
+    exclusive_options = [
+        ("favorite", "not_favorite"),
+        ("hidden", "not_hidden"),
+        ("title", "no_title"),
+        ("description", "no_description"),
+        ("only_photos", "only_movies"),
+        ("burst", "not_burst"),
+        ("live", "not_live"),
+        ("portrait", "not_portrait"),
+        ("screenshot", "not_screenshot"),
+        ("slow_mo", "not_slow_mo"),
+        ("time_lapse", "not_time_lapse"),
+        ("hdr", "not_hdr"),
+        ("selfie", "not_selfie"),
+        ("panorama", "not_panorama"),
+        ("export_by_date", "directory"),
+        ("export_as_hardlink", "exiftool"),
+        ("place", "no_place"),
+        ("deleted", "deleted_only"),
+        ("skip_edited", "skip_original_if_edited"),
+        ("export_as_hardlink", "convert_to_jpeg"),
+        ("export_as_hardlink", "download_missing"),
+        ("shared", "not_shared"),
+        ("has_comment", "no_comment"),
+        ("has_likes", "no_likes"),
+    ]
     try:
-        cfg.validate(cli=True)
+        cfg.validate(exclusive_options, cli=True)
-    except InvalidOptions as e:
+    except ConfigOptionsInvalidError as e:
-        click.echo(f"{e.message}")
+        click.echo(f"Incompatible export options: {e.message}", err=True)
+        raise click.Abort()
+
+    if missing and not download_missing:
+        click.echo(
+            "Incompatible export options: --missing must be used with --download-missing",
+            err=True,
+        )
         raise click.Abort()
 
     if save_config:
@@ -1697,7 +1740,9 @@ def export(
     # set defaults for options that need them
     jpeg_quality = DEFAULT_JPEG_QUALITY if jpeg_quality is None else jpeg_quality
     edited_suffix = DEFAULT_EDITED_SUFFIX if edited_suffix is None else edited_suffix
-    original_suffix = DEFAULT_ORIGINAL_SUFFIX if original_suffix is None else original_suffix
+    original_suffix = (
+        DEFAULT_ORIGINAL_SUFFIX if original_suffix is None else original_suffix
+    )
 
     # print(jpeg_quality, edited_suffix, original_suffix)
 
@@ -1711,52 +1756,6 @@ def export(
         click.echo(f"report is a directory, must be file name", err=True)
         raise click.Abort()
 
-    # sanity check input args
-    exclusive = [
-        (favorite, not_favorite),
-        (hidden, not_hidden),
-        (any(title), no_title),
-        (any(description), no_description),
-        (only_photos, only_movies),
-        (burst, not_burst),
-        (live, not_live),
-        (portrait, not_portrait),
-        (screenshot, not_screenshot),
-        (slow_mo, not_slow_mo),
-        (time_lapse, not_time_lapse),
-        (hdr, not_hdr),
-        (selfie, not_selfie),
-        (panorama, not_panorama),
-        (export_by_date, directory),
-        (export_as_hardlink, exiftool),
-        (any(place), no_place),
-        (deleted, deleted_only),
-        (skip_edited, skip_original_if_edited),
-        (export_as_hardlink, convert_to_jpeg),
-        (shared, not_shared),
-        (has_comment, no_comment),
-        (has_likes, no_likes),
-        (export_as_hardlink, cleanup),
-    ]
-    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
-
-    if export_as_hardlink and download_missing:
-        click.echo(
-            "Incompatible export options: --export-as-hardlink is not compatible with --download-missing",
-            err=True,
-        )
-        raise click.Abort()
-
-    if missing and not download_missing:
-        click.echo(
-            "Incompatible export options: --missing must be used with --download-missing",
-            err=True,
-        )
-        raise click.Abort()
-
     # if use_photokit and not check_photokit_authorization():
     #     click.echo(
     #         "Requesting access to use your Photos library. Click 'OK' on the dialog box to grant access."

osxphotos/configoptions.py

@@ -1,8 +1,8 @@
-""" Classes to load/save config settings for osxphotos CLI """
+""" ConfigOptions class to load/save config settings for osxphotos CLI """
 import toml
 
 
-class InvalidOptions(Exception):
+class ConfigOptionsException(Exception):
     """ Invalid combination of options. """
 
     def __init__(self, message):
@@ -10,16 +10,31 @@ class InvalidOptions(Exception):
         super().__init__(self.message)
 
 
-class OSXPhotosOptions:
+class ConfigOptionsInvalidError(ConfigOptionsException):
+    pass
+
+
+class ConfigOptionsLoadError(ConfigOptionsException):
+    pass
+
+
+class ConfigOptions:
     """ data class to store and load options for osxphotos commands """
 
-    def __init__(self, **kwargs):
+    def __init__(self, name, attrs, ignore=None):
-        args = locals()
+        """ init ConfigOptions class
 
-        self._attrs = {}
+        Args:
-        self._exclusive = []
+            name: name for these options, will be used for section heading in TOML file when saving/loading from file
+            attrs: dict with name and default value for all allowed attributes
+        """
+        self._name = name
+        self._attrs = attrs.copy()
+        if ignore:
+            for attrname in ignore:
+                self._attrs.pop(attrname, None)
 
-        self.set_attributes(args)
+        self.set_attributes(attrs)
 
     def set_attributes(self, args):
         for attr in self._attrs:
@@ -27,7 +42,7 @@ class OSXPhotosOptions:
                 arg = args[attr]
                 # don't test 'not arg'; need to handle empty strings as valid values
                 if arg is None or arg == False:
-                    if self._attrs[attr] == ():
+                    if type(self._attrs[attr]) == tuple:
                         setattr(self, attr, ())
                     else:
                         setattr(self, attr, self._attrs[attr])
@@ -36,7 +51,7 @@ class OSXPhotosOptions:
             except KeyError:
                 raise KeyError(f"Missing argument: {attr}")
 
-    def validate(self, cli=False):
+    def validate(self, exclusive, cli=False):
         """ validate combinations of otions
         
         Args:
@@ -49,14 +64,17 @@ class OSXPhotosOptions:
             InvalidOption if any combination of options is invalid
             InvalidOption.message will be descriptive message of invalid options
         """
+        if not exclusive:
+            return True
+
         prefix = "--" if cli else ""
-        for opt_pair in self._exclusive:
+        for opt_pair in exclusive:
             val0 = getattr(self, opt_pair[0])
             val1 = getattr(self, opt_pair[1])
             val0 = any(val0) if self._attrs[opt_pair[0]] == () else val0
             val1 = any(val1) if self._attrs[opt_pair[1]] == () else val1
             if val0 and val1:
-                raise InvalidOptions(
+                raise ConfigOptionsInvalidError(
                     f"{prefix}{opt_pair[0]} and {prefix}{opt_pair[1]} options cannot be used together"
                 )
         return True
@@ -78,234 +96,36 @@ class OSXPhotosOptions:
             data[attr] = val
 
         with open(filename, "w") as fd:
-            toml.dump({"export": data}, fd)
+            toml.dump({self._name: data}, fd)
 
-    def load_from_file(self, filename, override=None):
+    def load_from_file(self, filename, override=False):
         """ Load options from a TOML file.
 
         Args:
             filename: full path to TOML file
-            override: optional ExportOptions object; 
+            override: bool; if True, values in the TOML file will override values already set in the instance
-                      if provided, any value that's set in override will be used 
-                      to override what's in the TOML file
-        
-        Returns:
-            (ExportOptions, error): tuple of ExportOption object and error string; 
-            if there are any errors during the parsing of the TOML file, error will be set
-            to a descriptive error message otherwise it will be None
-        """
-        override = override or ExportOptions()
-        loaded = toml.load(filename)
-        options = ExportOptions()
-        if "export" not in loaded:
-            return options, f"[export] section missing from {filename}"
 
-        for attr in loaded["export"]:
+        Raises:
+            ConfigOptionsLoadError if there are any errors during the parsing of the TOML file
+        """
+        loaded = toml.load(filename)
+        name = self._name
+        if name not in loaded:
+            raise ConfigOptionsLoadError(f"[{name}] section missing from {filename}")
+
+        for attr in loaded[name]:
             if attr not in self._attrs:
-                return options, f"Unknown option: {attr}: {loaded['export'][attr]}"
+                raise ConfigOptionsLoadError(
-            val = loaded["export"][attr]
+                    f"Unknown option: {attr} = {loaded[name][attr]}"
-            val = getattr(override, attr) or val
+                )
-            if self._attrs[attr] == ():
+            val = loaded[name][attr]
+            if not override:
+                # use value from self if set
+                val = getattr(self, attr) or val
+            if type(self._attrs[attr]) == tuple:
                 val = tuple(val)
-            setattr(options, attr, val)
+            setattr(self, attr, val)
-        return options, None
+        return self, None
 
     def asdict(self):
         return {attr: getattr(self, attr) for attr in sorted(self._attrs.keys())}
-
-
-class ExportOptions(OSXPhotosOptions):
-    """ data class to store and load options for export command """
-
-    def __init__(
-        self,
-        db=None,
-        photos_library=None,
-        keyword=None,
-        person=None,
-        album=None,
-        folder=None,
-        uuid=None,
-        uuid_from_file=None,
-        title=None,
-        no_title=False,
-        description=None,
-        no_description=False,
-        uti=None,
-        ignore_case=False,
-        edited=False,
-        external_edit=False,
-        favorite=False,
-        not_favorite=False,
-        hidden=False,
-        not_hidden=False,
-        shared=False,
-        not_shared=False,
-        from_date=None,
-        to_date=None,
-        verbose=False,
-        missing=False,
-        update=True,
-        dry_run=False,
-        export_as_hardlink=False,
-        touch_file=False,
-        overwrite=False,
-        export_by_date=False,
-        skip_edited=False,
-        skip_original_if_edited=False,
-        skip_bursts=False,
-        skip_live=False,
-        skip_raw=False,
-        person_keyword=False,
-        album_keyword=False,
-        keyword_template=None,
-        description_template=None,
-        current_name=False,
-        convert_to_jpeg=False,
-        jpeg_quality=None,
-        sidecar=None,
-        only_photos=False,
-        only_movies=False,
-        burst=False,
-        not_burst=False,
-        live=False,
-        not_live=False,
-        download_missing=False,
-        exiftool=False,
-        ignore_date_modified=False,
-        portrait=False,
-        not_portrait=False,
-        screenshot=False,
-        not_screenshot=False,
-        slow_mo=False,
-        not_slow_mo=False,
-        time_lapse=False,
-        not_time_lapse=False,
-        hdr=False,
-        not_hdr=False,
-        selfie=False,
-        not_selfie=False,
-        panorama=False,
-        not_panorama=False,
-        has_raw=False,
-        directory=None,
-        filename_template=None,
-        edited_suffix=None,
-        original_suffix=None,
-        place=None,
-        no_place=False,
-        has_comment=False,
-        no_comment=False,
-        has_likes=False,
-        no_likes=False,
-        no_extended_attributes=False,
-        label=None,
-        deleted=False,
-        deleted_only=False,
-        use_photos_export=False,
-        use_photokit=False,
-        report=None,
-        cleanup=False,
-        **kwargs,
-    ):
-        args = locals()
-
-        # valid attributes and default values
-        self._attrs = {
-            "db": None,
-            "photos_library": (),
-            "keyword": (),
-            "person": (),
-            "album": (),
-            "folder": (),
-            "uuid": (),
-            "uuid_from_file": None,
-            "title": (),
-            "no_title": False,
-            "description": (),
-            "no_description": False,
-            "uti": None,
-            "ignore_case": False,
-            "edited": False,
-            "external_edit": False,
-            "favorite": False,
-            "not_favorite": False,
-            "hidden": False,
-            "not_hidden": False,
-            "shared": False,
-            "not_shared": False,
-            "from_date": None,
-            "to_date": None,
-            "verbose": False,
-            "missing": False,
-            "update": False,
-            "dry_run": False,
-            "export_as_hardlink": False,
-            "touch_file": False,
-            "overwrite": False,
-            "export_by_date": False,
-            "skip_edited": False,
-            "skip_original_if_edited": False,
-            "skip_bursts": False,
-            "skip_live": False,
-            "skip_raw": False,
-            "person_keyword": False,
-            "album_keyword": False,
-            "keyword_template": (),
-            "description_template": None,
-            "current_name": False,
-            "convert_to_jpeg": False,
-            "jpeg_quality": None,
-            "sidecar": (),
-            "only_photos": False,
-            "only_movies": False,
-            "burst": False,
-            "not_burst": False,
-            "live": False,
-            "not_live": False,
-            "download_missing": False,
-            "exiftool": False,
-            "ignore_date_modified": False,
-            "portrait": False,
-            "not_portrait": False,
-            "screenshot": False,
-            "not_screenshot": False,
-            "slow_mo": False,
-            "not_slow_mo": False,
-            "time_lapse": False,
-            "not_time_lapse": False,
-            "hdr": False,
-            "not_hdr": False,
-            "selfie": False,
-            "not_selfie": False,
-            "panorama": False,
-            "not_panorama": False,
-            "has_raw": False,
-            "directory": None,
-            "filename_template": None,
-            "edited_suffix": None,
-            "original_suffix": None,
-            "place": (),
-            "no_place": False,
-            "has_comment": False,
-            "no_comment": False,
-            "has_likes": False,
-            "no_likes": False,
-            "no_extended_attributes": False,
-            "label": (),
-            "deleted": False,
-            "deleted_only": False,
-            "use_photos_export": False,
-            "use_photokit": False,
-            "report": None,
-            "cleanup": False,
-        }
-
-        self._exclusive = [
-            ["favorite", "not_favorite"],
-            ["hidden", "not_hidden"],
-            ["title", "no_title"],
-            ["description", "no_description"],
-        ]
-
-        self.set_attributes(args)

tests/test_cli.py

@@ -875,7 +875,7 @@ def test_export_using_hardlinks_incompat_options():
                 "-V",
             ],
         )
-        assert result.exit_code == 0
+        assert result.exit_code == 1
         assert "Incompatible export options" in result.output
 
 
@@ -3961,26 +3961,3 @@ def test_export_cleanup():
         assert not pathlib.Path("./delete_me.txt").is_file()
         assert not pathlib.Path("./foo/delete_me_too.txt").is_file()
 
-
-def test_export_cleanup_export_as_hardling():
-    """ test export with incompatible option """
-    import os
-    import os.path
-    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, CLI_PHOTOS_DB),
-                ".",
-                "-V",
-                "--export-as-hardlink",
-                "--cleanup",
-            ],
-        )
-        assert "Incompatible export options" in result.output
-