Skip to content

Add helper to interpret items as key-value pairs (port from confit) - #50

Merged
sampsyo merged 2 commits into
beetbox:masterfrom
arcresu:confit-sync-pt2
May 30, 2019
Merged

Add helper to interpret items as key-value pairs (port from confit)#50
sampsyo merged 2 commits into
beetbox:masterfrom
arcresu:confit-sync-pt2

Conversation

@arcresu

@arcresu arcresu commented Apr 23, 2019

Copy link
Copy Markdown
Member

This brings the remaining feature from beetbox/beet's confit across to confuse. This feature was originally added for beets' fetchart plugin to support configuration like this:

fetchart:
    sources:
        - filesystem
        - coverart: release
        - itunes
        - coverart: releasegroup
        - '*'

This is meant to be read as a list of key-value pairs, where the value is optional. The above configuration is read using default_value='*' to result in the sequence of pairs:

[
    ('filesystem',  '*'           ),
    ('coverart',    'release'     ),
    ('itunes',      '*'           ),
    ('coverart',    'releasegroup'),
    ('*',           '*'           ),
]

wordofglass and others added 2 commits April 30, 2019 11:20
Original commits from beetbox/beets:

60bffbadbdee3652f65ff495a0436797a5296f71
Advanced fetchart source config: write (restore?) confit' as_pairs()

318f0c4d16710712ed49d10c621fbf52705161dd
Mon Sep 17 00:00:00 2001
wisp3rwind <17089248+wisp3rwind@users.noreply.github.com>
Advanced fetchart source config: pep8
@arcresu
arcresu marked this pull request as ready for review April 30, 2019 01:21
@arcresu

arcresu commented Apr 30, 2019

Copy link
Copy Markdown
Member Author

Below is the diff from confit to confuse after these changes are applied. It looks to me like it's only stuff that's been added to confuse but not confit, or changes that solve the same problems in different ways. This should be the final sweep from confit -> confuse. With this PR I can get beets' test suite to pass when re-exporting confuse as confit like in beetbox/beets#3224 (but with the backports removed).

diff --git a/../beets/beets/util/confit.py b/confuse.py
index a5e5225..3b7c080 100644
--- a/../beets/beets/util/confit.py
+++ b/confuse.py
@@ -17,20 +17,26 @@
 """
 from __future__ import division, absolute_import, print_function
 
+try:
+    import enum
+    SUPPORTS_ENUM = True
+except ImportError:
+    SUPPORTS_ENUM = False
+
+import argparse
+import optparse
 import platform
 import os
 import pkgutil
 import sys
 import yaml
 import re
-import six
 from collections import OrderedDict
-if six.PY2:
-    from collections import Mapping, Sequence
+if sys.version_info >= (3, 3):
+    from collections import abc
 else:
-    from collections.abc import Mapping, Sequence
+    import collections as abc
 
-UNIX_DIR_VAR = 'XDG_CONFIG_HOME'
 UNIX_DIR_FALLBACK = '~/.config'
 WINDOWS_DIR_VAR = 'APPDATA'
 WINDOWS_DIR_FALLBACK = '~\\AppData\\Roaming'
@@ -64,6 +70,21 @@ def iter_first(sequence):
         raise ValueError()
 
 
+def namespace_to_dict(obj):
+    """If obj is argparse.Namespace or optparse.Values we'll return
+      a dict representation of it, else return the original object.
+
+    Redefine this method if using other parsers.
+
+    :param obj: *
+    :return:
+    :rtype: dict or *
+    """
+    if isinstance(obj, (argparse.Namespace, optparse.Values)):
+        return vars(obj)
+    return obj
+
+
 # Exceptions.
 
 class ConfigError(Exception):
@@ -247,20 +268,86 @@ class ConfigView(object):
     def __contains__(self, key):
         return self[key].exists()
 
-    def set_args(self, namespace):
+    @classmethod
+    def _build_namespace_dict(cls, obj, dots=False):
+        """Recursively replaces all argparse.Namespace and optparse.Values
+        with dicts and drops any keys with None values.
+
+        Additionally, if dots is True, will expand any dot delimited
+        keys.
+
+        :param obj: Namespace, Values, or dict to iterate over. Other
+            values will simply be returned.
+        :type obj: argparse.Namespace or optparse.Values or dict or *
+        :param dots: If True, any properties on obj that contain dots (.)
+            will be broken down into child dictionaries.
+        :return: A new dictionary or the value passed if obj was not a
+            dict, Namespace, or Values.
+        :rtype: dict or *
+        """
+        # We expect our root object to be a dict, but it may come in as
+        # a namespace
+        obj = namespace_to_dict(obj)
+        # We only deal with dictionaries
+        if not isinstance(obj, dict):
+            return obj
+
+        # Get keys iterator
+        keys = obj.keys() if PY3 else obj.iterkeys()
+        if dots:
+            # Dots needs sorted keys to prevent parents from
+            # clobbering children
+            keys = sorted(list(keys))
+
+        output = {}
+        for key in keys:
+            value = obj[key]
+            if value is None:  # Avoid unset options.
+                continue
+
+            save_to = output
+            result = cls._build_namespace_dict(value, dots)
+            if dots:
+                # Split keys by dots as this signifies nesting
+                split = key.split('.')
+                if len(split) > 1:
+                    # The last index will be the key we assign result to
+                    key = split.pop()
+                    # Build the dict tree if needed and change where
+                    # we're saving to
+                    for child_key in split:
+                        if child_key in save_to and \
+                                isinstance(save_to[child_key], dict):
+                            save_to = save_to[child_key]
+                        else:
+                            # Clobber or create
+                            save_to[child_key] = {}
+                            save_to = save_to[child_key]
+
+            # Save
+            if key in save_to:
+                save_to[key].update(result)
+            else:
+                save_to[key] = result
+        return output
+
+    def set_args(self, namespace, dots=False):
         """Overlay parsed command-line arguments, generated by a library
-        like argparse or optparse, onto this view's value. ``namespace``
-        can be a ``dict`` or namespace object.
+        like argparse or optparse, onto this view's value.
+
+        :param namespace: Dictionary or Namespace to overlay this config with.
+            Supports nested Dictionaries and Namespaces.
+        :type namespace: dict or Namespace
+        :param dots: If True, any properties on namespace that contain dots (.)
+            will be broken down into child dictionaries.
+            :Example:
+
+            {'foo.bar': 'car'}
+            # Will be turned into
+            {'foo': {'bar': 'car'}}
+        :type dots: bool
         """
-        args = {}
-        if isinstance(namespace, dict):
-            items = namespace.items()
-        else:
-            items = namespace.__dict__.items()
-        for key, value in items:
-            if value is not None:  # Avoid unset options.
-                args[key] = value
-        self.set(args)
+        self.set(self._build_namespace_dict(namespace, dots))
 
     # Magical conversions. These special methods make it possible to use
     # View objects somewhat transparently in certain circumstances. For
@@ -413,13 +500,13 @@ class ConfigView(object):
 
     def as_str_seq(self, split=True):
         """Get the value as a sequence of strings. Equivalent to
-        `get(StrSeq())`.
+        `get(StrSeq(split=split))`.
         """
         return self.get(StrSeq(split=split))
 
     def as_pairs(self, default_value=None):
         """Get the value as a sequence of pairs of two strings. Equivalent to
-        `get(Pairs())`.
+        `get(Pairs(default_value=default_value))`.
         """
         return self.get(Pairs(default_value=default_value))
 
@@ -579,6 +666,21 @@ def _package_path(name):
     return os.path.dirname(os.path.abspath(filepath))
 
 
+def xdg_config_dirs():
+    """Returns a list of paths taken from the XDG_CONFIG_DIRS
+    and XDG_CONFIG_HOME environment varibables if they exist
+    """
+    paths = []
+    if 'XDG_CONFIG_HOME' in os.environ:
+        paths.append(os.environ['XDG_CONFIG_HOME'])
+    if 'XDG_CONFIG_DIRS' in os.environ:
+        paths.extend(os.environ['XDG_CONFIG_DIRS'].split(':'))
+    else:
+        paths.append('/etc/xdg')
+    paths.append('/etc')
+    return paths
+
+
 def config_dirs():
     """Return a platform-specific list of candidates for user
     configuration directories on the system.
@@ -592,8 +694,7 @@ def config_dirs():
     if platform.system() == 'Darwin':
         paths.append(MAC_DIR)
         paths.append(UNIX_DIR_FALLBACK)
-        if UNIX_DIR_VAR in os.environ:
-            paths.append(os.environ[UNIX_DIR_VAR])
+        paths.extend(xdg_config_dirs())
 
     elif platform.system() == 'Windows':
         paths.append(WINDOWS_DIR_FALLBACK)
@@ -603,8 +704,7 @@ def config_dirs():
     else:
         # Assume Unix.
         paths.append(UNIX_DIR_FALLBACK)
-        if UNIX_DIR_VAR in os.environ:
-            paths.append(os.environ[UNIX_DIR_VAR])
+        paths.extend(xdg_config_dirs())
 
     # Expand and deduplicate paths.
     out = []
@@ -702,11 +802,11 @@ class Dumper(yaml.SafeDumper):
         for item_key, item_value in mapping:
             node_key = self.represent_data(item_key)
             node_value = self.represent_data(item_value)
-            if not (isinstance(node_key, yaml.ScalarNode) and
-                    not node_key.style):
+            if not (isinstance(node_key, yaml.ScalarNode)
+                    and not node_key.style):
                 best_style = False
-            if not (isinstance(node_value, yaml.ScalarNode) and
-                    not node_value.style):
+            if not (isinstance(node_value, yaml.ScalarNode)
+                    and not node_value.style):
                 best_style = False
             value.append((node_key, node_value))
         if flow_style is None:
@@ -801,7 +901,10 @@ class Configuration(RootView):
 
         # Resolve default source location. We do this ahead of time to
         # avoid unexpected problems if the working directory changes.
-        self._package_path = _package_path(appname)
+        if self.modname:
+            self._package_path = _package_path(self.modname)
+        else:
+            self._package_path = None
 
         self._env_var = '{0}DIR'.format(self.appname.upper())
 
@@ -868,11 +971,14 @@ class Configuration(RootView):
 
         else:
             # Search platform-specific locations. If no config file is
-            # found, fall back to the final directory in the list.
-            for confdir in config_dirs():
+            # found, fall back to the first directory in the list.
+            configdirs = config_dirs()
+            for confdir in configdirs:
                 appdir = os.path.join(confdir, self.appname)
                 if os.path.isfile(os.path.join(appdir, CONFIG_FILENAME)):
                     break
+            else:
+                appdir = os.path.join(configdirs[0], self.appname)
 
         # Ensure that the directory exists.
         if not os.path.isdir(appdir):
@@ -924,7 +1030,7 @@ class Configuration(RootView):
             with open(default_source.filename, 'rb') as fp:
                 default_data = fp.read()
             yaml_out = restore_yaml_comments(yaml_out,
-                                             default_data.decode('utf8'))
+                                             default_data.decode('utf-8'))
 
         return yaml_out
 
@@ -1108,6 +1214,28 @@ class MappingTemplate(Template):
         return 'MappingTemplate({0})'.format(repr(self.subtemplates))
 
 
+class Sequence(Template):
+    """A template used to validate lists of similar items,
+    based on a given subtemplate.
+    """
+    def __init__(self, subtemplate):
+        """Create a template for a list with items validated
+        on a given subtemplate.
+        """
+        self.subtemplate = as_template(subtemplate)
+
+    def value(self, view, template=None):
+        """Get a list of items validated against the template.
+        """
+        out = []
+        for item in view:
+            out.append(self.subtemplate.value(item, self))
+        return out
+
+    def __repr__(self):
+        return 'Sequence({0})'.format(repr(self.subtemplate))
+
+
 class String(Template):
     """A string configuration value template.
     """
@@ -1148,28 +1276,44 @@ class String(Template):
 class Choice(Template):
     """A template that permits values from a sequence of choices.
     """
-    def __init__(self, choices):
+    def __init__(self, choices, default=REQUIRED):
         """Create a template that validates any of the values from the
         iterable `choices`.
 
         If `choices` is a map, then the corresponding value is emitted.
         Otherwise, the value itself is emitted.
+
+        If `choices` is a `Enum`, then the enum entry with the value is
+        emitted.
         """
+        super(Choice, self).__init__(default)
         self.choices = choices
 
     def convert(self, value, view):
         """Ensure that the value is among the choices (and remap if the
         choices are a mapping).
         """
+        if (SUPPORTS_ENUM and isinstance(self.choices, type)
+                and issubclass(self.choices, enum.Enum)):
+            try:
+                return self.choices(value)
+            except ValueError:
+                self.fail(
+                    u'must be one of {0!r}, not {1!r}'.format(
+                        [c.value for c in self.choices], value
+                    ),
+                    view
+                )
+
         if value not in self.choices:
             self.fail(
-                u'must be one of {0}, not {1}'.format(
-                    repr(list(self.choices)), repr(value)
+                u'must be one of {0!r}, not {1!r}'.format(
+                    list(self.choices), value
                 ),
                 view
             )
 
-        if isinstance(self.choices, Mapping):
+        if isinstance(self.choices, abc.Mapping):
             return self.choices[value]
         else:
             return value
@@ -1245,14 +1389,14 @@ class StrSeq(Template):
     Validates both actual YAML string lists and single strings. Strings
     can optionally be split on whitespace.
     """
-    def __init__(self, split=True):
+    def __init__(self, split=True, default=REQUIRED):
         """Create a new template.
 
         `split` indicates whether, when the underlying value is a single
         string, it should be split on whitespace. Otherwise, the
         resulting value is a list containing a single string.
         """
-        super(StrSeq, self).__init__()
+        super(StrSeq, self).__init__(default)
         self.split = split
 
     def _convert_value(self, x, view):
@@ -1278,7 +1422,6 @@ class StrSeq(Template):
             except TypeError:
                 self.fail(u'must be a whitespace-separated string or a list',
                           view, True)
-
         return [self._convert_value(v, view) for v in value]
 
 
@@ -1310,11 +1453,11 @@ class Pairs(StrSeq):
             return (super(Pairs, self)._convert_value(x, view),
                     self.default_value)
         except ConfigTypeError:
-            if isinstance(x, Mapping):
+            if isinstance(x, abc.Mapping):
                 if len(x) != 1:
                     self.fail(u'must be a single-element mapping', view, True)
                 k, v = iter_first(x.items())
-            elif isinstance(x, Sequence):
+            elif isinstance(x, abc.Sequence):
                 if len(x) != 2:
                     self.fail(u'must be a two-element list', view, True)
                 k, v = x
@@ -1371,7 +1514,7 @@ class Filename(Template):
         return 'Filename({0})'.format(', '.join(args))
 
     def resolve_relative_to(self, view, template):
-        if not isinstance(template, (Mapping, MappingTemplate)):
+        if not isinstance(template, (abc.Mapping, MappingTemplate)):
             # disallow config.get(Filename(relative_to='foo'))
             raise ConfigTemplateError(
                 u'relative_to may only be used when getting multiple values.'
@@ -1412,7 +1555,7 @@ class Filename(Template):
                     ).format(view.name, self.relative_to))
                 else:
                     raise ConfigTemplateError((
-                        u'missing template for {0}, needed to expand {1}\'s' +
+                        u'missing template for {0}, needed to expand {1}\'s'
                         u'relative path'
                     ).format(self.relative_to, view.name))
 
@@ -1490,7 +1633,7 @@ def as_template(value):
     if isinstance(value, Template):
         # If it's already a Template, pass it through.
         return value
-    elif isinstance(value, Mapping):
+    elif isinstance(value, abc.Mapping):
         # Dictionaries work as templates.
         return MappingTemplate(value)
     elif value is int:
@@ -1504,6 +1647,9 @@ def as_template(value):
     elif isinstance(value, set):
         # convert to list to avoid hash related problems
         return Choice(list(value))
+    elif (SUPPORTS_ENUM and isinstance(value, type)
+            and issubclass(value, enum.Enum)):
+        return Choice(value)
     elif isinstance(value, list):
         return OneOf(value)
     elif value is float:
@@ -1511,9 +1657,9 @@ def as_template(value):
     elif value is None:
         return Template()
     elif value is dict:
-        return TypeTemplate(Mapping)
+        return TypeTemplate(abc.Mapping)
     elif value is list:
-        return TypeTemplate(Sequence)
+        return TypeTemplate(abc.Sequence)
     elif isinstance(value, type):
         return TypeTemplate(value)
     else:

@sampsyo

sampsyo commented May 30, 2019

Copy link
Copy Markdown
Member

Looks great to me! I'll merge this now.

In retrospect, I'm not sure why I put Confuse under my username instead of the beetbox organization. Perhaps just because it doesn't directly have to do with music & audio, but that's not a terribly good reason. It might be a good idea to move it over there so we can collaborate on maintaining it.

@sampsyo
sampsyo merged commit aab17bb into beetbox:master May 30, 2019
@arcresu
arcresu deleted the confit-sync-pt2 branch May 31, 2019 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants