Eh/basic fixes

CHANGES.txt

@@ -1,3 +1,4 @@
+v0.4.0, 08/05/21 -- Layer constructor accepts optional model argument; improved arg and kwarg name handling; additional bug fix, documentation, and testing improvements
 v0.3.1, 09/02/20 -- Ensure files required to run tests get installed
 v0.3.0, 09/02/20 -- Stack.load and CLI functions can look in alternate locations for Layers; ArgMode, Layer and Stack can be imported directly from layerstack; Stack.save more robust to different object types; testing and documentation improvements
 v0.2.0, 10/30/19 -- adding layerstack_stack CLI, ArgMode can be set with str, bug and documentation fixes

LICENSE

@@ -1,4 +1,4 @@
-Copyright (c) 2020 Alliance for Sustainable Energy, LLC, All Rights Reserved
+Copyright (c) 2021 Alliance for Sustainable Energy, LLC, All Rights Reserved
 
 Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:
 

layerstack/args.py

@@ -100,6 +100,11 @@ def is_list(self):
         """
         return self._is_list
 
+    def get_name(self, cleaned=True):
+        if cleaned:
+            return self.name.replace("-", "_")
+        return self.name
+
     def _set_value(self, value):
         """
         Called by derived classes to set their _value attribute.
@@ -505,15 +510,16 @@ def set_args(self, cli_args):
             unexpected behavior.
         """
         if not self.mode == ArgMode.USE:
-            raise LayerStackRuntimeError("{} ".format(self.__class__.__name__) + 
+            raise LayerStackRuntimeError(f"{self.__class__.__name__} "
                 "must be in ArgMode.USE to set values.")
         try:
             self.mode = ArgMode.DESC
             for arg in self:
                 try:
-                    temp_value = eval('cli_args.' + arg.name) 
+                    temp_value = getattr(cli_args, arg.get_name(cleaned=False))
                 except Exception as e:
-                    raise LayerStackRuntimeError("{} not found in cli_args, {}".format(arg.name,e))
+                    raise LayerStackRuntimeError(f"{arg.get_name(cleaned=False)} not found in "
+                        f"cli_args:\n{cli_args},\nbecause {e}")
                 arg.value = temp_value # may throw if temp_value is bad per arg.parser, etc.
             self.mode = ArgMode.USE
         except Exception as e:
@@ -621,7 +627,7 @@ def items(self):
             (if mode == ArgMode.DESC), or its .value (if mode == ArgMode.USE)
         """
         if self.mode == ArgMode.USE:
-            return [(name, kwarg.value) for name, kwarg in super().items()]
+            return [(kwarg.get_name(), kwarg.value) for kwarg in super().values()]
         return super().items()
 
     def iteritems(self):
@@ -636,7 +642,7 @@ def iteritems(self):
             ArgMode.USE)
         """
         for name, kwarg in super().items():
-            yield (name, kwarg) if self.mode == ArgMode.DESC else (name, kwarg.value)
+            yield (name, kwarg) if self.mode == ArgMode.DESC else (kwarg.get_name(), kwarg.value)
 
     def values(self):
         """
@@ -726,30 +732,8 @@ def add_arguments(self, parser, short_names=[]):
             raise LayerStackRuntimeError("{} ".format(self.__class__.__name__) + 
                 "must be in ArgMode.DESC to add arguments to an argparse parser.")
 
-        def get_short_name(name):
-            short_name = None; n = 0
-
-            # first try splitting on '_' to get good letters
-            n = 0
-            chars = "".join([x[0] for x in name.split('_')])
-            while not short_name:
-                n += 1
-                if chars[:n] not in short_names:
-                    short_name = chars[:n]
-
-            # now just use all characters
-            n = 1
-            while not short_name:
-                n += 1
-                if name[:n] not in short_names:
-                    short_name = name[:n]
-
-            assert short_name
-            short_names.append(short_name)
-            return short_name
-
         for name, kwarg in self.items():
-            short_name = get_short_name(name)
+            short_name = get_short_name(name, short_names=short_names)
             kwarg_kwargs = kwarg.add_argument_kwargs()
             try:
                 parser.add_argument('-' + short_name, '--' + name,
@@ -790,12 +774,67 @@ def set_kwargs(self, cli_args):
             self.mode = ArgMode.DESC
             for key, kwarg in self.items():
                 try:
-                    temp_value = eval('cli_args.' + key) 
+                    temp_value = getattr(cli_args, kwarg.get_name())
                 except Exception as e:
-                    raise LayerStackRuntimeError("{} not found in cli_args, {}".format(key,e))
+                    raise LayerStackRuntimeError(f"{kwarg.get_name()} not found in "
+                        f"cli_args:\n{cli_args},\nbecause {e}")
                 kwarg.value = temp_value # may throw if temp_value is bad per kwarg.parser, etc.
             self.mode = ArgMode.USE
         except Exception as e:
             self.mode = ArgMode.USE
             raise e
-
+
+
+def get_short_name(name, short_names = [], seps = ['_', '-']):
+    short_name = None
+
+    def multisplit(astr, seps):
+        parts = None
+        for sep in seps:
+            if parts is None:
+                parts = astr.split(sep)
+                continue
+            tmp = []
+            for part in parts:
+                tmp.extend(part.split(sep))
+            parts = tmp
+        return parts
+
+    parts = multisplit(name, seps)
+    len_parts = [len(part) for part in parts]
+
+    M = 1; N = len(parts); candidates = set(); k = 0
+    logger.debug(parts)
+    while not short_name:
+        if k and (len(candidates) == k):
+            raise LayerStackRuntimeError("Unable to find a short name "
+                "for {name}. Current short names:\n[\n  {short_names}\n]"
+                "\nStarted with parts = {parts},\nevaluated "
+                "candidates:\n[\n  {candidates}\n]".format(
+                    name = name, 
+                    short_names = ",\n  ".join([repr(sn) for sn in short_names]),
+                    parts = parts,
+                    candidates = ",\n  ".join([repr(candi) for candi in candidates])
+                ))
+        n = 1; k = len(candidates)
+        logger.debug(f"M = {M}; N = {N}; k = {k}")
+        while n <= N:
+            candidate = ''
+            for i in range(n):
+                m = min(len_parts[i], M)
+                candidate += parts[i][:m]
+                logger.debug(f"  n = {n}; i = {i}; m = {m}; {candidate}")
+            for i in range(n,N):
+                mm1 = min(len_parts[i], M - 1)
+                candidate += parts[i][:mm1]
+                logger.debug(f"  n = {n}; N = {N}; i = {i}; mm1 = {mm1}; {candidate}")
+            if candidate not in short_names:
+                short_name = candidate
+                break
+            candidates.add(candidate)
+            n += 1
+        M += 1
+
+    assert short_name
+    short_names.append(short_name)
+    return short_name

layerstack/layer.py

@@ -133,8 +133,7 @@ def main(cls, log_format=DEFAULT_LOG_FORMAT):
         arg_list = cls.args()
         arg_list.add_arguments(parser)
         kwarg_dict = cls.kwargs()
-        # *** issue 23, likely just need to add an h to this list
-        kwarg_dict.add_arguments(parser, short_names=['r', 'd'])
+        kwarg_dict.add_arguments(parser, short_names=['r', 'd', 'h'])
 
         # Parse args and set values        
         cli_args = parser.parse_args()
@@ -200,7 +199,6 @@ def _main_apply(cls, cli_args, arg_list, kwarg_dict):
         assert arg_list.mode == ArgMode.USE
         assert kwarg_dict.mode == ArgMode.USE
         from layerstack.stack import Stack
-        # TODO: Fix KwargDict so **kwarg_dict works natively
         return cls.apply(Stack(run_dir=cli_args.run_dir), 
             *arg_list, **{k: v for k, v in kwarg_dict.items()})
 
@@ -444,13 +442,13 @@ def create(cls, name, parent_dir, desc=None, layer_base_class=LayerBase):
 
         Parameters
         ----------
-        name : 'str'
+        name : str
             Layer name
-        parent_dir : 'str'
+        parent_dir : str or pathlib.Path
             Parent directory for layer
-        desc : 'str'
+        desc : str
             Layer description
-        layer_base_class : 'LayerBase|ModelLayerBase'
+        layer_base_class : LayerBase or child class
             Base class on which to build layer
 
         Returns
@@ -460,6 +458,7 @@ def create(cls, name, parent_dir, desc=None, layer_base_class=LayerBase):
         """
 
         # Create the directory
+        parent_dir = Path(parent_dir)
         if not parent_dir.exists():
             raise LayerStackError(f"The parent_dir {parent_dir} does not exist.") # maynot need the msg_begin here
         dir_name = name.lower().replace(" ", "_")

layerstack/stack.py

@@ -26,7 +26,8 @@
 from __future__ import print_function, division, absolute_import
 
 import argparse
-from collections import MutableSequence, OrderedDict
+from collections import OrderedDict
+from collections.abc import MutableSequence
 import json
 import logging
 from pathlib import Path
@@ -695,7 +696,7 @@ def run(self, save_path=None, log_level=logging.INFO, archive=True):
             end_file_log(logfile)
         except:
             chdir(old_cur_dir)
-            logger.info(f"Stack failed after {timer_str(timer() - start)}")
+            logger.error(f"Stack failed after {timer_str(timer() - start)}")
             end_file_log(logfile)
             raise        
 

layerstack/tests/__init__.py

@@ -1,7 +1,20 @@
 # -*- coding: utf-8 -*-
-
+import subprocess
 from pathlib import Path
 
 here = Path(__file__).parent
 outdir = here / 'outputs'
 layer_library_dir = here / 'layer_library'
+
+def get_output_str(stdout_stderr):
+    if not isinstance(stdout_stderr, bytes):
+        stdout_stderr = stdout_stderr.read() # _io.BufferedReader
+    return stdout_stderr.decode('ascii').rstrip()
+
+def run_command(args, logger, test_name, msg_postfix):
+    ret = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    stdout, stderr = ret.communicate()
+    stdout = get_output_str(stdout); stderr = get_output_str(stderr)
+    logger.debug(f"In {test_name}, stdout:\n{stdout}\nstderr:\n{stderr}\n"
+        f"returncode = {ret.returncode}\n{msg_postfix}")
+    return ret, stdout, stderr

layerstack/tests/layer_library/test_kwarg_name_clashes/layer.py

@@ -0,0 +1,72 @@
+from __future__ import print_function, division, absolute_import
+
+from builtins import super
+import logging
+from uuid import UUID
+
+from layerstack.args import Arg, Kwarg
+from layerstack.layer import LayerBase
+
+logger = logging.getLogger('layerstack.layers.TestManyKwargNameClashes')
+
+
+class TestKwargNameClashes(LayerBase):
+    name = "Test kwarg name clashes"
+    uuid = UUID("88cfcb69-27c0-4664-a295-c909a61ad832")
+    version = '0.1.0'
+    desc = None
+
+    @classmethod
+    def args(cls, model=None):
+        arg_list = super().args()
+        return arg_list
+
+    @classmethod
+    def kwargs(cls):
+        '''
+        Each layer must define its keyword arguments by populating and returning
+        a KwargDict object.
+
+        Returns
+        -------
+        KwargDict
+            KwargDict object describing the layer's keyword arguments. Keyword
+            argument specifications in the apply method should match what is 
+            defined in this method (i.e., be equivalent to 
+            Kwarg.name=Kwarg.default).
+        '''
+        kwarg_dict = super().kwargs()
+        kwarg_dict['hit_rate'] = Kwarg(
+            description="Kwargs starting with h should be allowed")
+        kwarg_dict['hearth_rug_dog'] = Kwarg(
+            description="Short name should be -hrd"
+        )
+        kwarg_dict['heart_rate'] = Kwarg(
+            description="Short name should be -her"
+        )
+        kwarg_dict['herself_running_dearly'] = Kwarg(
+            description="Look deep for a name that works"
+        )
+        return kwarg_dict
+
+    @classmethod
+    def apply(cls, stack, hit_rate = None, hearth_rug_dog = None, 
+              heart_rate = None, herself_running_dearly = None):
+        """
+        No logic required--just testing kwarg-passing.
+        """
+        return True
+
+
+if __name__ == '__main__':
+    # Single-layer command-line interface entry point.
+
+    # Parameters
+    # ----------
+    # log_format : str
+    #     custom logging format to use with the logging package via 
+    #     layerstack.start_console_log
+    # 
+    TestKwargNameClashes.main()
+
+