Final -- Improve handling of nested dependencies (#181)

* fixes issues wrt more than 2 levels of class nesting references. was ssuming to fall back on defaults instead of recursing the config space to set the correct parameters. * linted * updated docs for cmd line overrides on nested classes * fixed edge case from #152 where the default attrs object wasn't getting recursed to set config arg values within nested classes. * adding test conf file * wonky patch to deal with the monster under the bed that is #152. refactor should be thought about to handle all the parent/child relations in a cleaner manner * WIP: Changing all the nesting deps to be handle via a DAG. Breaks most tests. * fixes optional class refs. * fix signature to deal with tuner -- should resolve all tests in /tune. Probably a bunch of vestigial code still to remove -- base tests still only 43/48 * fixing some tests (#163) * fixing some tests * some interesting work but breaking the tests * test_command_line pass * some cleaning and refactoring * fix when optional nested value is None * clearner refactor to handle type optionals * "config" cannot be a general argument * fix some more tests and windows path compatible * s3 tests pass on windows * all tests now are passing * dont need the graph in builde space * renamed everything to builder_space and doc string * some cleaning Co-authored-by: mbelanger_explorance <[email protected]> * Re-Improve handling of nested dependencies (#180) * Nested values edge case fix (#160) * fixes issues wrt more than 2 levels of class nesting references. was ssuming to fall back on defaults instead of recursing the config space to set the correct parameters. * linted * updated docs for cmd line overrides on nested classes * fixed edge case from #152 where the default attrs object wasn't getting recursed to set config arg values within nested classes. * adding test conf file * fixing some tests * some interesting work but breaking the tests * test_command_line pass * some cleaning and refactoring * fix when optional nested value is None * clearner refactor to handle type optionals * "config" cannot be a general argument * fix some more tests and windows path compatible * s3 tests pass on windows * all tests now are passing * dont need the graph in builde space * renamed everything to builder_space and doc string * some cleaning * removing networkx dep * linted * removed dataclasses dep by swapping to a namedtuple * fix-up of some tests. some were missing correct spock classes as *args. Some now raise a different exception with the new refactor * moved help functionality to separate file for readability of the builder class * documentation. almost finished * finished doc strings. linted * files for extra tests * fixing issues with python3.6 which doesn't have the typing alias 'list' yet only 'typing.List' * linted Co-authored-by: mbelanger_explorance <[email protected]> * updated readme Co-authored-by: gbmarc1 <[email protected]> Co-authored-by: mbelanger_explorance <[email protected]>
fidelity · Dec 13, 2021 · ba4330c · ba4330c
1 parent 3ac6bcd
commit ba4330c
Show file tree

Hide file tree

Showing 27 changed files with 1,692 additions and 803 deletions.
diff --git a/README.md b/README.md
@@ -66,22 +66,19 @@ Example `spock` usage is located [here](https://github.com/fidelity/spock/blob/m
 
 See [Releases](https://github.com/fidelity/spock/releases) for more information.
 
+#### December 13, 2021
+* Refactored the backend to better handle nested dependencies (and for clarity)
+
 #### August 17, 2021
 * Added hyper-parameter tuning backend support for Ax via Service API
 
 #### July 21, 2021
 * Added hyper-parameter tuning support with `pip install spock-config[tune]`
 * Hyper-parameter tuning backend support for Optuna define-and-run API (WIP for Ax)
 
-#### May 6th, 2021
-* Added S3 support with `pip install spock-config[s3]`
-* S3 addon supports automatically handling loading/saving from paths defined with `s3://` URI(s) by passing in an
-active `boto3.Session`
-
-
 ## Original Implementation
 
 [Nicholas Cilfone](https://github.com/ncilfone), [Siddharth Narayanan](https://github.com/sidnarayanan)
 ___
-`spock` is developed and maintained by the **Artificial Intelligence Center of Excellence at Fidelity Investments**.
+`spock` was originally developed by the **Artificial Intelligence Center of Excellence at Fidelity Investments**.
 
diff --git a/REQUIREMENTS.txt b/REQUIREMENTS.txt
@@ -1,4 +1,5 @@
 attrs==21.2.0
 GitPython==3.1.18
 pytomlpp==1.0.3
-pyYAML==6.0
+pyYAML==6.0
+networkx
diff --git a/spock/addons/tune/builder.py b/spock/addons/tune/builder.py
@@ -22,28 +22,6 @@ def __init__(self, *args, **kwargs):
         """
         super().__init__(*args, module_name="spock.addons.tune.config", **kwargs)
 
-    def _handle_arguments(self, args, class_obj):
-        """Ovverides base -- Handles all argument mapping
-
-        Creates a dictionary of named parameters that are mapped to the final type of object
-
-        *Args*:
-
-            args: read file arguments
-            class_obj: instance of a class obj
-
-        *Returns*:
-
-            fields: dictionary of mapped parameters
-
-        """
-        attr_name = class_obj.__name__
-        fields = {
-            val.name: val.type(**args[attr_name][val.name])
-            for val in class_obj.__attrs_attrs__
-        }
-        return fields
-
     @staticmethod
     def _make_group_override_parser(parser, class_obj, class_name):
         """Makes a name specific override parser for a given class obj

diff --git a/spock/args.py b/spock/args.py
@@ -3,7 +3,193 @@
 # Copyright FMR LLC <[email protected]>
 # SPDX-License-Identifier: Apache-2.0
 
-"""Handles import aliases to allow backwards compat with backends"""
+"""Handles mapping config arguments to a payload with both general and class specific sets"""
 
-# from spock.backend.dataclass.args import *
-from spock.backend.typed import SavePath
+
+from _warnings import warn
+
+from spock.graph import Graph
+
+
+class SpockArguments:
+    """Class that handles mapping the read parameter dictionary to general or class level arguments
+
+    Attributes:
+        _arguments: dictionary of arguments
+
+    """
+
+    def __init__(self, arguments: dict, config_dag: Graph):
+        """Init call for SpockArguments class
+
+        Handles creating a clean arguments dictionary that can be cleanly mapped to spock classes
+
+        Args:
+            arguments: dictionary of parameters from the config file(s)
+            config_dag: graph of the dependencies between spock classes
+
+        """
+        general_arguments = self._get_general_arguments(arguments, config_dag)
+        attribute_name_to_config_name_mapping = (
+            self._attribute_name_to_config_name_mapping(config_dag, general_arguments)
+        )
+        self._arguments = self._clean_arguments(arguments, general_arguments)
+        self._assign_general_arguments_to_config(
+            general_arguments, attribute_name_to_config_name_mapping
+        )
+
+    def __getitem__(self, key: str):
+        """Gets value from the _arguments dictionary
+
+        Args:
+            key: dictionary key
+
+        Returns:
+            argument at the specified key
+
+        """
+        return self._arguments[key]
+
+    def __iter__(self):
+        """Returns the next value of the keys within the _arguments dictionary
+
+        Returns:
+            current key for the _arguments dictionary
+
+        """
+        for key in self._arguments:
+            yield key
+
+    @property
+    def items(self):
+        """Returns the k,v tuple iterator for the _arguments dictionary"""
+        return self._arguments.items()
+
+    @property
+    def keys(self):
+        """Returns an iterator for the keys of the _arguments dictionary"""
+        return self._arguments.keys()
+
+    @property
+    def values(self):
+        """Returns an iterator for the values of the _arguments dictionary"""
+        return self._arguments.values()
+
+    @staticmethod
+    def _get_general_arguments(arguments: dict, config_dag: Graph):
+        """Creates a dictionary of config file parameters that are defined at the general level (not class specific)
+
+        Args:
+            arguments: dictionary of parameters from the config file(s)
+            config_dag: graph of the dependencies between spock classes
+
+        Returns:
+            dictionary of general level parameters
+
+        """
+        config_names = {n.__name__ for n in config_dag.nodes}
+        return {
+            key: value
+            for key, value in arguments.items()
+            if key not in config_names and key != "config"
+        }
+
+    def _attribute_name_to_config_name_mapping(
+        self, config_dag: Graph, general_arguments: dict
+    ):
+        """Returns a mapping of names to spock config class parameter names
+
+        Args:
+            config_dag: graph of the dependencies between spock classes
+            general_arguments: dictionary of arguments at the general level
+
+        Returns:
+            dictionary of parameters mapped to spock classes
+
+        """
+        attribute_name_to_config_name_mapping = {}
+        for n in config_dag.nodes:
+            for attr in n.__attrs_attrs__:
+                if attr.name in general_arguments:
+                    if self._is_duplicated_key(
+                        attribute_name_to_config_name_mapping, attr.name, n.__name__
+                    ):
+                        raise SpockDuplicateArgumentError(
+                            f"`{attr.name}` key is located in more than one config and cannot be resolved automatically."
+                            f"Either specify the config name (`<config>.{attr.name}`) or change the key name in the config."
+                        )
+                    attribute_name_to_config_name_mapping[attr.name] = n.__name__
+
+        return attribute_name_to_config_name_mapping
+
+    @staticmethod
+    def _is_duplicated_key(
+        attribute_name_to_config_name_mapping: dict, attr_name: str, config_name: str
+    ):
+        """Checks if a duplicated key exists in multiple classes
+
+        Args:
+            attribute_name_to_config_name_mapping: dictionary of class specific name mappings
+            attr_name:
+            config_name:
+
+        Returns:
+            boolean if duplicated
+
+        """
+        return (
+            attr_name in attribute_name_to_config_name_mapping
+            and attribute_name_to_config_name_mapping[attr_name] != config_name
+        )
+
+    def _assign_general_arguments_to_config(
+        self, general_arguments: dict, attribute_name_to_config_name_mapping: dict
+    ):
+        """Assigns the values from general definitions to values within specific classes if the specific definition
+        doesn't exist
+
+        Args:
+            general_arguments:
+            attribute_name_to_config_name_mapping:
+
+        Returns:
+            None
+        """
+        for arg, value in general_arguments.items():
+            config_name = attribute_name_to_config_name_mapping[arg]
+            if config_name in self._arguments:
+                # Specific arguments supersede general arguments
+                if arg not in self._arguments[config_name]:
+                    self._arguments[config_name][arg] = value
+                else:
+                    warn(
+                        f"Ignoring general argument `{arg}` for config `{config_name}`\n"
+                        f"Specific argument value preceded general arguments.",
+                        SyntaxWarning,
+                    )
+            else:
+                self._arguments[config_name] = {arg: value}
+
+    @staticmethod
+    def _clean_arguments(arguments: dict, general_arguments: dict):
+        """Sets up a clean dictionary for those not in the general dictionary
+
+        Args:
+            arguments: dictionary of all arguments
+            general_arguments: dictionary of general level arguments
+
+        Returns:
+            clean dictionary of parameters not at the general level
+
+        """
+        clean_arguments = {}
+        for arg, value in arguments.items():
+            if arg not in general_arguments:
+                clean_arguments[arg] = value
+        return clean_arguments
+
+
+class SpockDuplicateArgumentError(Exception):
+    """Custom exception type for duplicated values"""
+
+    pass
diff --git a/spock/backend/__init__.py b/spock/backend/__init__.py
@@ -9,4 +9,16 @@
 Please refer to the documentation provided in the README.md
 """
 
-__all__ = ["builder", "config", "payload", "saver", "typed"]
+__all__ = [
+    "builder",
+    "config",
+    "field_handlers",
+    "handler",
+    "help",
+    "payload",
+    "saver",
+    "spaces",
+    "typed",
+    "utils",
+    "wrappers",
+]