Repository: glotzerlab/signac
Pull request #860 (2022-11-06 23:32:19)

Refactor _Config class.
## Description
Refactors `_Config` to be private and have a minimal API. Also makes a few helper functions private.

## Checklist:
- [x] I am familiar with the [Contributing Guidelines](https://github.com/glotzerlab/signac/blob/master/CONTRIBUTING.md).
- [x] I agree with the terms of the [Contributor Agreement](https://github.com/glotzerlab/signac/blob/master/ContributorAgreement.md).
- [x] My name is on the [list of contributors](https://github.com/glotzerlab/signac/blob/master/contributors.yaml).
- [x] The changes introduced by this pull request are covered by existing or newly introduced tests.
- [ ] The [package documentation](https://github.com/glotzerlab/signac/tree/master/doc) and [framework documentation](https://docs.signac.io/) in [signac-docs](https://github.com/glotzerlab/signac-docs) are up to date with these changes.
- [x] I have updated the [changelog](https://github.com/glotzerlab/signac/blob/master/changelog.txt) and added any related issue and pull request numbers for future reference.


codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/860?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) Report
> Merging [#860](https://codecov.io/gh/glotzerlab/signac/pull/860?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (e6aec94) into [master](https://codecov.io/gh/glotzerlab/signac/commit/fdb0790e174f5387f31a4b9feda90294f73dd999?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (fdb0790) will **increase** coverage by `0.00%`.
> The diff coverage is `64.51%`.

```diff
@@           Coverage Diff           @@
##           master     #860   +/-   ##
=======================================
  Coverage   78.57%   78.58%           
=======================================
  Files          49       49           
  Lines        4579     4576    -3     
  Branches      905      904    -1     
=======================================
- Hits         3598     3596    -2     
  Misses        807      807           
+ Partials      174      173    -1     
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/860?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) | Coverage Δ | |
|---|---|---|
| [signac/\_\_main\_\_.py](https://codecov.io/gh/glotzerlab/signac/pull/860/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL19fbWFpbl9fLnB5) | `77.05% <56.00%> (ø)` | |
| [signac/common/config.py](https://codecov.io/gh/glotzerlab/signac/pull/860/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbW1vbi9jb25maWcucHk=) | `90.47% <100.00%> (+1.08%)` | :arrow_up: |
| [signac/contrib/project.py](https://codecov.io/gh/glotzerlab/signac/pull/860/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvcHJvamVjdC5weQ==) | `82.90% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)


Patch:
diff --git a/changelog.txt b/changelog.txt
index 5a3e7199..3733d03d 100644
--- a/changelog.txt
+++ b/changelog.txt
@@ -60,6 +60,7 @@ Removed
  - The public API of the ``synced_collection`` subpackage (#807, #790).
  - The warnings module (#815).
  - The signac.common.validate module (#853).
+ - The ``Config`` class (#860).
  - The ``DictManager`` class (#858).
 
 Version 1
diff --git a/signac/__main__.py b/signac/__main__.py
index 3be5790b..388f6430 100644
--- a/signac/__main__.py
+++ b/signac/__main__.py
@@ -133,15 +133,15 @@ def _open_job_by_id(project, job_id):
         )
 
 
-def find_with_filter_or_none(args):
+def _find_with_filter_or_none(args):
     """Return a filtered subset of jobs or None."""
     if args.job_id or args.filter:
-        return find_with_filter(args)
+        return _find_with_filter(args)
     else:
         return None
 
 
-def find_with_filter(args):
+def _find_with_filter(args):
     """Return a filtered subset of jobs."""
     if getattr(args, "job_id", None):
         if args.filter:
@@ -194,7 +194,7 @@ def main_statepoint(args):
 def main_document(args):
     """Handle document subcommand."""
     project = get_project()
-    for job_id in find_with_filter(args):
+    for job_id in _find_with_filter(args):
         job = _open_job_by_id(project, job_id)
         if args.pretty:
             pprint(job.document(), depth=args.pretty)
@@ -276,7 +276,7 @@ def main_find(args):
             return pformat(s, depth=args.pretty)
 
     try:
-        for job_id in find_with_filter(args):
+        for job_id in _find_with_filter(args):
             print(job_id)
             job = project.open_job(id=job_id)
 
@@ -304,7 +304,7 @@ def main_diff(args):
     """Handle diff subcommand."""
     project = get_project()
 
-    jobs = find_with_filter_or_none(args)
+    jobs = _find_with_filter_or_none(args)
     jobs = (
         (_open_job_by_id(project, job) for job in jobs) if jobs is not None else project
     )
@@ -322,7 +322,7 @@ def main_view(args):
     project.create_linked_view(
         prefix=args.prefix,
         path=args.path,
-        job_ids=find_with_filter(args),
+        job_ids=_find_with_filter(args),
     )
 
 
@@ -337,7 +337,7 @@ def main_schema(args):
     project = get_project()
     print(
         project.detect_schema(
-            exclude_const=args.exclude_const, subset=find_with_filter_or_none(args)
+            exclude_const=args.exclude_const, subset=_find_with_filter_or_none(args)
         ).format(
             depth=args.depth, precision=args.precision, max_num_range=args.max_num_range
         )
@@ -393,7 +393,7 @@ def main_sync(args):
     except LookupError:
         _print_err("WARNING: The destination does not appear to be a project path.")
         raise
-    selection = find_with_filter_or_none(args)
+    selection = _find_with_filter_or_none(args)
 
     if args.strategy:
         if args.strategy[0].isupper():
@@ -597,7 +597,7 @@ def main_export(args):
     copytree = shutil.move if args.move else None
 
     project = get_project()
-    jobs = [project.open_job(id=job_id) for job_id in find_with_filter(args)]
+    jobs = [project.open_job(id=job_id) for job_id in _find_with_filter(args)]
 
     paths = {}
     with tqdm(total=len(jobs), desc="Export") as pbar:
@@ -659,27 +659,6 @@ def main_migrate(args):
         apply_migrations(root)
 
 
-def verify_config(cfg, preserve_errors=True):
-    """Verify provided configuration."""
-    verification = cfg.verify(preserve_errors=preserve_errors)
-    if verification is True:
-        _print_err("Passed.")
-    else:
-        for entry in flatten_errors(cfg, verification):
-            # each entry is a tuple
-            section_list, key, error = entry
-            if key is not None:
-                section_list.append(key)
-            else:
-                section_list.append("[missing section]")
-            section_string = ".".join(section_list)
-            if error is False:
-                error = "Possibly invalid or missing."
-            else:
-                error = type(error).__name__
-            _print_err(" ".join((section_string, ":", error)))
-
-
 def main_config_show(args):
     """Handle config show subcommand."""
     cfg = None
@@ -709,10 +688,31 @@ def main_config_show(args):
     if not isinstance(cfg, Section):
         print(cfg)
     else:
-        for line in config.Config(cfg).write():
+        for line in config._Config(cfg).write():
             print(line)
 
 
+def _verify_config(cfg, preserve_errors=True):
+    """Verify provided configuration."""
+    verification = cfg.verify(preserve_errors=preserve_errors)
+    if verification is True:
+        _print_err("Passed.")
+    else:
+        for entry in flatten_errors(cfg, verification):
+            # each entry is a tuple
+            section_list, key, error = entry
+            if key is not None:
+                section_list.append(key)
+            else:
+                section_list.append("[missing section]")
+            section_string = ".".join(section_list)
+            if error is False:
+                error = "Possibly invalid or missing."
+            else:
+                error = type(error).__name__
+            _print_err(" ".join((section_string, ":", error)))
+
+
 def main_config_verify(args):
     """Handle config verify subcommand."""
     cfg = None
@@ -737,7 +737,7 @@ def main_config_verify(args):
     else:
         if cfg.filename is not None:
             _print_err(f"Verification of config file '{cfg.filename}'.")
-        verify_config(cfg)
+        _verify_config(cfg)
 
 
 def main_config_set(args):
@@ -789,7 +789,7 @@ def main_shell(args):
         print("No project within this directory.")
         print("If you want to initialize a project, execute `$ signac init`.")
     else:
-        _jobs = find_with_filter(args)
+        _jobs = _find_with_filter(args)
 
         def jobs():
             for id_ in _jobs:
diff --git a/signac/common/config.py b/signac/common/config.py
index d4543a9b..18c67db8 100644
--- a/signac/common/config.py
+++ b/signac/common/config.py
@@ -97,10 +97,18 @@ def _locate_config_dir(search_path):
             search_path = up
 
 
+class _Config(ConfigObj):
+    """Manages configuration for a signac project."""
+
+    def verify(self, *, preserve_errors=False):
+        """Validate the contents of this configuration."""
+        return super().validate(Validator(), preserve_errors=preserve_errors)
+
+
 def _read_config_file(filename):
     logger.debug(f"Reading config file '{filename}'.")
     try:
-        config = Config(filename, configspec=_CFG.split("\n"))
+        config = _Config(filename, configspec=_CFG.split("\n"))
     except (OSError, ConfigObjError) as error:
         raise ConfigError(f"Failed to read configuration file '{filename}':\n{error}")
     verification = config.verify()
@@ -125,14 +133,14 @@ def _load_config(path=None):
 
     Returns
     --------
-    :class:`Config`
+    :class:`_Config`
         The composite configuration including both project-local and global
         config data if requested. Note that because this config is a composite,
         modifications to the returned value will not be reflected in the files.
     """
     if path is None:
         path = os.getcwd()
-    config = Config(configspec=_CFG.split("\n"))
+    config = _Config(configspec=_CFG.split("\n"))
 
     # Add in any global or user config files. For now this only finds user-specific
     # files, but it could be updated in the future to support e.g. system-wide config files.
@@ -143,15 +151,3 @@ def _load_config(path=None):
     if os.path.isfile(_get_project_config_fn(path)):
         config.merge(_read_config_file(_get_project_config_fn(path)))
     return config
-
-
-class Config(ConfigObj):
-    """Manages configuration for a signac project."""
-
-    encoding = "utf-8"
-
-    def verify(self, validator=None, *args, **kwargs):
-        """Validate the contents of this configuration."""
-        if validator is None:
-            validator = Validator()
-        return super().validate(validator, *args, **kwargs)
diff --git a/signac/contrib/project.py b/signac/contrib/project.py
index 7d44460d..4576bda2 100644
--- a/signac/contrib/project.py
+++ b/signac/contrib/project.py
@@ -22,7 +22,7 @@ from threading import RLock
 
 from .._synced_collections.backends.collection_json import BufferedJSONAttrDict
 from ..common.config import (
-    Config,
+    _Config,
     _get_project_config_fn,
     _load_config,
     _locate_config_dir,
@@ -51,7 +51,7 @@ JOB_ID_LENGTH = 32
 JOB_ID_REGEX = re.compile(f"[a-f0-9]{{{JOB_ID_LENGTH}}}")
 
 
-class _ProjectConfig(Config):
+class _ProjectConfig(_Config):
     r"""Extends the project config to make it immutable.
 
     Parameters


Test patch:
diff --git a/tests/test_shell.py b/tests/test_shell.py
index efa70676..c675f6a0 100644
--- a/tests/test_shell.py
+++ b/tests/test_shell.py
@@ -744,17 +744,17 @@ class TestBasicShell:
         self.call("python -m signac init".split())
         out = self.call("python -m signac config --local show".split()).strip()
         cfg = config._read_config_file(".signac/config")
-        expected = config.Config(cfg).write()
+        expected = config._Config(cfg).write()
         assert out.split(os.linesep) == expected
 
         out = self.call("python -m signac config show".split()).strip()
         cfg = config._load_config()
-        expected = config.Config(cfg).write()
+        expected = config._Config(cfg).write()
         assert out.split(os.linesep) == expected
 
         out = self.call("python -m signac config --global show".split()).strip()
         cfg = config._read_config_file(config.USER_CONFIG_FN)
-        expected = config.Config(cfg).write()
+        expected = config._Config(cfg).write()
         assert out.split(os.linesep) == expected
 
     def test_config_set(self):