Repository: NetherlandsForensicInstitute/confidence (language: python)

Issue:
Remove separator argument?
The dot is the one and only separator that makes sense, really. The only place where another separator is used is loading environment variables, but that's done outside of `Configuration` or `_merge`. Types and functions now often pass around a separator, might be better to just assume that's always going to be a dot and hardcode it throughout.


Discussion / hints:

Fix patch:
diff --git a/CHANGES.md b/CHANGES.md
index 0ff83a8..4f5c91f 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -4,6 +4,7 @@ Changes
 development (master)
 --------------------
 
+- Remove configurable key separator, hardcode the default.
 - Rename enumeration values (like `Locality.USER`) to be upper case.
 
 0.9 (2021-02-01)
diff --git a/confidence/models.py b/confidence/models.py
index f664232..f8ef9ef 100644
--- a/confidence/models.py
+++ b/confidence/models.py
@@ -33,18 +33,15 @@ class Configuration(Mapping):
 
     def __init__(self,
                  *sources: typing.Mapping[str, typing.Any],
-                 separator: str = '.',
                  missing: typing.Any = Missing.SILENT):
         """
         Create a new `.Configuration`, based on one or multiple source mappings.
 
         :param sources: source mappings to base this `.Configuration` on,
             ordered from least to most significant
-        :param separator: the character(s) to use as the separator between keys
         :param missing: policy to be used when a configured key is missing,
             either as a `.Missing` instance or a default value
         """
-        self._separator = separator
         self._missing = missing
         self._root = self
 
@@ -60,13 +57,11 @@ def __init__(self,
                     source = source._source
 
                 # merge values from source into self._source, overwriting any corresponding keys
-                merge(self._source,
-                      split_keys(source, separator=self._separator, colliding=_COLLIDING_KEYS),
-                      conflict=Conflict.OVERWRITE)
+                merge(self._source, split_keys(source, colliding=_COLLIDING_KEYS), conflict=Conflict.OVERWRITE)
 
     def _wrap(self, value: typing.MutableMapping[str, typing.Any]) -> 'Configuration':
         # create an instance of our current type, copying 'configured' properties / policies
-        namespace = type(self)(separator=self._separator, missing=self._missing)
+        namespace = type(self)(missing=self._missing)
         namespace._source = value
         # carry the root object from namespace to namespace, references are always resolved from root
         namespace._root = self._root
@@ -122,8 +117,7 @@ def get(self,
         Gets a value for the specified path.
 
         :param path: the configuration key to fetch a value for, steps
-            separated by the separator supplied to the constructor (default
-            ``.``)
+            separated by a dot (``.``)
         :param default: a value to return if no value is found for the
             supplied path (``None`` is allowed)
         :param as_type: an optional callable to apply to the value found for
@@ -139,7 +133,7 @@ def get(self,
         steps_taken = []
         try:
             # walk through the values dictionary
-            for step in path.split(self._separator):
+            for step in path.split('.'):
                 steps_taken.append(step)
                 value = value[step]
 
@@ -165,7 +159,7 @@ def get(self,
             if default is not NoDefault:
                 return default
             else:
-                missing_key = self._separator.join(steps_taken)
+                missing_key = '.'.join(steps_taken)
                 raise NotConfiguredError(f'no configuration for key {missing_key}', key=missing_key) from e
 
     def __getattr__(self, attr: str) -> typing.Any:
diff --git a/confidence/utils.py b/confidence/utils.py
index e460be1..39eef21 100644
--- a/confidence/utils.py
+++ b/confidence/utils.py
@@ -51,20 +51,19 @@ def merge(left: typing.MutableMapping[str, typing.Any],
 
 
 def split_keys(mapping: typing.Mapping[str, typing.Any],
-               separator: str = '.',
                colliding: typing.Optional[typing.Container] = None) -> typing.Mapping[str, typing.Any]:
     """
-    Recursively walks *mapping* to split keys that contain the separator into
-    nested mappings.
+    Recursively walks *mapping* to split keys that contain a dot into nested
+    mappings.
 
     .. note::
 
         Keys not of type `str` are not supported and will raise errors.
 
     :param mapping: the mapping to process
-    :param separator: the character (sequence) to use as the separator between
-        keys
-    :return: a mapping where keys containing *separator* are split into nested
+    :param colliding: a container of keys (names) that should be triggering a
+        warning that they collide with other functionality
+    :return: a mapping where keys containing a dot are split into nested
         mappings
     """
     result: typing.MutableMapping[str, typing.Any] = {}
@@ -72,18 +71,18 @@ def split_keys(mapping: typing.Mapping[str, typing.Any],
     for key, value in mapping.items():
         if isinstance(value, Mapping):
             # recursively split key(s) in value
-            value = split_keys(value, separator)
+            value = split_keys(value)
 
         # reject non-str keys, avoid complicating access patterns
         if not isinstance(key, str):
             raise ValueError(f'non-str type keys ({key}, {key.__class__.__module__}.{key.__class__.__name__}) '
                              'not supported')
 
-        if separator in key:
-            # update key to be the first part before the separator
-            key, rest = key.split(separator, 1)
+        if '.' in key:
+            # update key to be the first part before the dot separator
+            key, rest = key.split('.', 1)
             # use rest as the new key of value, recursively split that and update value
-            value = split_keys({rest: value}, separator)
+            value = split_keys({rest: value})
 
         if colliding and key in colliding:
             # warn about configured keys colliding with Configuration members


Test patch:
diff --git a/tests/test_defaults.py b/tests/test_defaults.py
index 6948191..a1113ab 100644
--- a/tests/test_defaults.py
+++ b/tests/test_defaults.py
@@ -1,8 +1,8 @@
-from confidence import Configuration
+from confidence import Configuration, NotConfigured
 
 
 def test_constructor_defaults():
     subject = Configuration()
-    assert subject._separator == '.'
+    assert subject._missing == NotConfigured
     assert len(subject) == len(subject._source) == 0
     assert list(subject) == []
diff --git a/tests/test_namespace.py b/tests/test_namespace.py
index 45b4684..bae018d 100644
--- a/tests/test_namespace.py
+++ b/tests/test_namespace.py
@@ -49,16 +49,15 @@ def test_not_configured():
 
 def test_collisions():
     with patch('confidence.utils.warnings') as warnings:
-        subject = Configuration({'key': 'value', 'keys': [1, 2], '_separator': '_'})
+        subject = Configuration({'key': 'value', 'keys': [1, 2], '_missing': 'error'})
 
-    for collision in ('keys', '_separator'):
+    for collision in ('keys', '_missing'):
         warnings.warn.assert_any_call(f'key {collision} collides with a named member, use get() method to '
                                       f'retrieve the value for {collision}',
                                       UserWarning)
 
     assert subject.key == 'value'
     assert callable(subject.keys)
-    assert subject._separator == '.'
 
 
 def test_dir():