Repository: skypyproject/skypy
Pull request #245 (2020-08-05 11:59:15)

simplify config file syntax
## Description

Replace the `module` and `function` syntax in the driver with fully qualified `module.function` syntax. All arguments can now be given in the `args` list or dict. Literal strings must be escaped by an initial `~` (tilde).

## Checklist
- [x] Follow the [Contributor Guidelines](https://github.com/skypyproject/skypy/blob/master/CONTRIBUTING.md)
- [x] Write unit tests
- [x] Write documentation strings
- [x] Assign someone from your working team to review this pull request
- [x] Assign someone from the infrastructure team to review this pull request


skypybot: Hello @ntessore! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:



* In the file [`skypy/pipeline/tests/test_driver.py`](https://github.com/skypyproject/skypy/blob/7f75363a738ff05117338a4b476959870a3325a6/skypy/pipeline/tests/test_driver.py):

> [Line 57:80](https://github.com/skypyproject/skypy/blob/7f75363a738ff05117338a4b476959870a3325a6/skypy/pipeline/tests/test_driver.py#L57): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (85 > 79 characters)
> [Line 63:80](https://github.com/skypyproject/skypy/blob/7f75363a738ff05117338a4b476959870a3325a6/skypy/pipeline/tests/test_driver.py#L63): [E501](https://duckduckgo.com/?q=pep8%20E501) line too long (85 > 79 characters)



Patch:
diff --git a/examples/abundance_matching.yml b/examples/abundance_matching.yml
index 6fef7fd..ad6ffc8 100644
--- a/examples/abundance_matching.yml
+++ b/examples/abundance_matching.yml
@@ -1,25 +1,19 @@
 cosmology:
-  module: astropy.cosmology
-  function: default_cosmology.get
+  function: astropy.cosmology.default_cosmology.get
 A_s: 2.1982e-09
 n_s: 0.969453
 redshift: 1
 growth_function:
-  module: skypy.power_spectrum
-  function: growth_function
-  requires:
+  function: skypy.power_spectrum.growth_function
+  args:
     redshift: redshift
     cosmology: cosmology
 wavenumber:
-  module: numpy
-  function: logspace
+  function: numpy.logspace
   args: [-4, 2, 100]
-  kwargs:
-    base: 10
 power_spectrum:
-  module: skypy.power_spectrum
-  function: eisenstein_hu
-  requires:
+  function: skypy.power_spectrum.eisenstein_hu
+  args:
     wavenumber: wavenumber
     A_s: A_s
     n_s: n_s
@@ -27,34 +21,29 @@ power_spectrum:
 tables:
   halos:
     mass:
-      module: skypy.halo.mass
-      function: press_schechter
-      kwargs:
+      function: skypy.halo.mass.press_schechter
+      args:
         m_min: 1.0E+9
         m_max: 1.0E+12
         resolution: 1000
         size: 1000
-      requires:
         wavenumber: wavenumber
         power_spectrum: power_spectrum
         growth_function: growth_function
         cosmology: cosmology
   galaxies:
     luminosity:
-      module: skypy.galaxy.luminosity
-      function: herbel_luminosities
-      kwargs:
+      function: skypy.galaxy.luminosity.herbel_luminosities
+      args:
         alpha: -0.5
         a_m: -0.610
         b_m: -20.416
         size: 100
-      requires:
         redshift: redshift
   abundance_matching:
     init:
-      module: skypy.halo.abundance_matching
-      function: vale_ostriker
-      requires:
+      function: skypy.halo.abundance_matching.vale_ostriker
+      args:
         halos: halos
         galaxies: galaxies
       depends: [halos.complete, galaxies.complete]
diff --git a/examples/gravitational_wave_rates.yml b/examples/gravitational_wave_rates.yml
index 6b4f6c7..eced095 100644
--- a/examples/gravitational_wave_rates.yml
+++ b/examples/gravitational_wave_rates.yml
@@ -1,50 +1,39 @@
 cosmology:
-  module: astropy.cosmology
-  function: default_cosmology.get
+  function: astropy.cosmology.default_cosmology.get
 tables:
   merger_rates:
     redshift:
-      module: skypy.galaxy.redshift
-      function: herbel_redshift
-      kwargs:
+      function: skypy.galaxy.redshift.herbel_redshift
+      args:
         alpha: -0.5
         a_phi: -2.232
         b_phi: 0.0141
         a_m: -0.610
         b_m: -20.416
         size: 1000
-      requires:
         cosmology: cosmology
     luminosity:
-      module: skypy.galaxy.luminosity
-      function: herbel_luminosities
-      kwargs:
+      function: skypy.galaxy.luminosity.herbel_luminosities
+      args:
         alpha: -0.5
         a_m: -0.610
         b_m: -20.416
-      requires:
         redshift: merger_rates.redshift
     ns_ns_rate:
-      module: skypy.gravitational_wave
-      function: b_band_merger_rate
-      kwargs:
-        population: NS-NS
-        optimism: low
-      requires:
+      function: skypy.gravitational_wave.b_band_merger_rate
+      args:
+        population: '''NS-NS'''
+        optimism: '''low'''
         luminosity: merger_rates.luminosity
     ns_bh_rate:
-      module: skypy.gravitational_wave
-      function: b_band_merger_rate
-      kwargs:
-        population: NS-BH
-        optimism: low
-      requires:
+      function: skypy.gravitational_wave.b_band_merger_rate
+      args:
+        population: '''NS-BH'''
+        optimism: '''low'''
         luminosity: merger_rates.luminosity
     bh_bh_rate:
-      module: skypy.gravitational_wave
-      function: b_band_merger_rate
-      kwargs:
-        population: BH-BH
-        optimism: low
-      requires:
+      function: skypy.gravitational_wave.b_band_merger_rate
+      args:
+        population: '''BH-BH'''
+        optimism: '''low'''
         luminosity: merger_rates.luminosity
diff --git a/examples/herbel_galaxies.yaml b/examples/herbel_galaxies.yaml
index 6f8c520..73a4ee2 100644
--- a/examples/herbel_galaxies.yaml
+++ b/examples/herbel_galaxies.yaml
@@ -1,7 +1,6 @@
 cosmology:
-  module: astropy.cosmology
-  function: FlatLambdaCDM
-  kwargs:
+  function: astropy.cosmology.FlatLambdaCDM
+  args:
     H0: 67.7
     Om0: 0.307
 red_galaxy_sersic_index: 4
@@ -9,117 +8,95 @@ blue_galaxy_sersic_index: 1
 tables:
   red_galaxies:
     redshift:
-      module: skypy.galaxy.redshift
-      function: herbel_redshift
-      kwargs:
+      function: skypy.galaxy.redshift.herbel_redshift
+      args:
         alpha: -0.5
         a_phi: -2.232
         b_phi: 0.0141
         a_m: -0.610
         b_m: -20.416
         size: 141
-      requires:
         cosmology: cosmology
     luminosity:
-      module: skypy.galaxy.luminosity
-      function: herbel_luminosities
-      kwargs:
+      function: skypy.galaxy.luminosity.herbel_luminosities
+      args:
         alpha: -0.5
         a_m: -0.610
         b_m: -20.416
-      requires:
         redshift: red_galaxies.redshift
     magnitude:
-      module: skypy.utils.astronomy
-      function: absolute_magnitude_from_luminosity
-      requires:
+      function: skypy.utils.astronomy.absolute_magnitude_from_luminosity
+      args:
         luminosity: red_galaxies.luminosity
     spectral_coefficients:
-      module: skypy.galaxy.spectrum
-      function: dirichlet_coefficients
-      kwargs:
+      function: skypy.galaxy.spectrum.dirichlet_coefficients
+      args:
         alpha0: [2.461, 2.358, 2.568, 2.268, 2.402]
         alpha1: [2.410, 2.340, 2.200, 2.540, 2.464]
-      requires:
         redshift: red_galaxies.redshift
     physical_size:
-      module: skypy.galaxy.size
-      function: linear_lognormal
-      kwargs:
+      function: skypy.galaxy.size.linear_lognormal
+      args:
         a_mu: -0.243
         b_mu: 0.954
         sigma: 0.568
-      requires:
         magnitude: red_galaxies.magnitude
     angular_size:
-      module: skypy.galaxy.size
-      function: angular_size
-      requires:
+      function: skypy.galaxy.size.angular_size
+      args:
         physical_size: red_galaxies.physical_size
         redshift: red_galaxies.redshift
         cosmology: cosmology
     ellipticity:
-      module: skypy.galaxy.ellipticity
-      function: beta_ellipticity
-      kwargs:
+      function: skypy.galaxy.ellipticity.beta_ellipticity
+      args:
         e_ratio: 0.45
         e_sum: 3.5
         size: 141
   blue_galaxies:
     redshift:
-      module: skypy.galaxy.redshift
-      function: herbel_redshift
-      kwargs:
+      function: skypy.galaxy.redshift.herbel_redshift
+      args:
         alpha: -1.3
         a_phi: -0.264
         b_phi: 0.0063
         a_m: -0.417
         b_m: -20.591
         size: 63
-      requires:
         cosmology: cosmology
     luminosity:
-      module: skypy.galaxy.luminosity
-      function: herbel_luminosities
-      kwargs:
+      function: skypy.galaxy.luminosity.herbel_luminosities
+      args:
         alpha: -1.3
         a_m: -0.417
         b_m: -20.591
-      requires:
         redshift: blue_galaxies.redshift
     magnitude:
-      module: skypy.utils.astronomy
-      function: absolute_magnitude_from_luminosity
-      requires:
+      function: skypy.utils.astronomy.absolute_magnitude_from_luminosity
+      args:
         luminosity: blue_galaxies.luminosity
     spectral_coefficients:
-      module: skypy.galaxy.spectrum
-      function: dirichlet_coefficients
-      kwargs:
+      function: skypy.galaxy.spectrum.dirichlet_coefficients
+      args:
         alpha0: [2.079, 3.524, 1.917, 1.992, 2.536]
         alpha1: [2.265, 3.862, 1.921, 1.685, 2.480]
-      requires:
         redshift: blue_galaxies.redshift
     physical_size:
-      module: skypy.galaxy.size
-      function: linear_lognormal
-      kwargs:
+      function: skypy.galaxy.size.linear_lognormal
+      args:
         a_mu: -0.243
         b_mu: 0.954
         sigma: 0.568
-      requires:
         magnitude: blue_galaxies.magnitude
     angular_size:
-      module: skypy.galaxy.size
-      function: angular_size
-      requires:
+      function: skypy.galaxy.size.angular_size
+      args:
         physical_size: blue_galaxies.physical_size
         redshift: blue_galaxies.redshift
         cosmology: cosmology
     ellipticity:
-      module: skypy.galaxy.ellipticity
-      function: beta_ellipticity
-      kwargs:
+      function: skypy.galaxy.ellipticity.beta_ellipticity
+      args:
         e_ratio: 0.45
         e_sum: 3.5
         size: 63
diff --git a/skypy/pipeline/driver.py b/skypy/pipeline/driver.py
index 36872d8..2298564 100644
--- a/skypy/pipeline/driver.py
+++ b/skypy/pipeline/driver.py
@@ -6,6 +6,8 @@ and handle their results.
 
 from collections.abc import Mapping
 from copy import deepcopy
+from importlib import import_module
+import builtins
 import networkx
 import re
 
@@ -15,6 +17,23 @@ __all__ = [
 ]
 
 
+def _items(a):
+    '''return keys and values for dict or list'''
+    if hasattr(a, 'items'):
+        return a.items()
+    return enumerate(a)
+
+
+def _is_string_argument(s):
+    # string arguments are enclosed in either single or double quotation marks
+    return s[0] in '"\'' and s[0] == s[-1]
+
+
+def _parse_string_argument(s):
+    # strip nested quotes from literal string
+    return s[1:-1]
+
+
 class SkyPyDriver:
     r'''Class for running pipelines.
 
@@ -45,15 +64,14 @@ class SkyPyDriver:
         -----
         Each step in the pipeline is configured by a dictionary specifying:
 
-        - 'function' : the name of the function
-        - 'module' : the name of the the module to import 'function' from
-        - 'args' : a list of positional arguments (by value)
-        - 'kwargs' : a dictionary of keyword arguments
-        - 'requires' : a dictionary of keyword arguments
+        - 'function' : the fully qualified name of the function
+        - 'args' : a list of positional arguments (by value), or a dictionary
+                   of keyword arguments
 
-        Note that 'kwargs' specifices keyword arguments by value, wheras
-        'requires' specifices the names of previous steps in the pipeline and
-        uses their return values as keyword arguments.
+        Note that 'args' either specifices keyword arguments by value, or the
+        names of previous steps in the pipeline and uses their return values as
+        keyword arguments. Literal strings (i.e. not field names) are escaped
+        by nested quotes: '"this is a literal string"' and "'another one'".
 
         'configuration' should contain the name and configuration of each
         variable and/or an entry named 'tables'. 'tables' should contain a set
@@ -76,7 +94,7 @@ class SkyPyDriver:
         # table_config contains settings for all table columns
         config = deepcopy(configuration)
         table_config = config.pop('tables', {})
-        default_table = {'module': 'astropy.table', 'function': 'Table'}
+        default_table = {'function': 'astropy.table.Table'}
         config.update({k: v.pop('init', default_table)
                       for k, v in table_config.items()})
 
@@ -85,7 +103,7 @@ class SkyPyDriver:
 
         # Variables initialised by value don't require function evaluations
         def isfunction(f):
-            return isinstance(f, Mapping) and 'module' in f and 'function' in f
+            return isinstance(f, Mapping) and 'function' in f
         variables = {k: v for k, v in config.items() if not isfunction(v)}
         for v in variables:
             dag.add_node(v)
@@ -102,10 +120,15 @@ class SkyPyDriver:
                 dag.add_node(job)
 
         # Add edges for all requirements and dependencies
+        def deps(settings):
+            deps = settings.get('depends', [])
+            args = settings.get('args', [])
+            for k, v in _items(args):
+                if isinstance(v, str) and not _is_string_argument(v):
+                    deps.append(v)
+            return deps
         for job, settings in config.items():
-            dependencies = settings.get('depends', [])
-            dependencies += settings.get('requires', {}).values()
-            dag.add_edges_from((d, job) for d in dependencies)
+            dag.add_edges_from((d, job) for d in deps(settings))
         for table, columns in table_config.items():
             table_complete = '.'.join((table, 'complete'))
             dag.add_edge(table, table_complete)
@@ -113,9 +136,7 @@ class SkyPyDriver:
                 job = '.'.join((table, column))
                 dag.add_edge(table, job)
                 dag.add_edge(job, table_complete)
-                dependencies = settings.get('depends', [])
-                dependencies += settings.get('requires', {}).values()
-                dag.add_edges_from((d, job) for d in dependencies)
+                dag.add_edges_from((d, job) for d in deps(settings))
 
         # Execute jobs in order that resolves dependencies
         for job in networkx.topological_sort(dag):
@@ -138,25 +159,36 @@ class SkyPyDriver:
     def _call_from_config(self, config):
 
         # Import function
-        module_name = config.get('module')
-        object_name, function_name = re.search(r'^(\w*?)\.?(\w*)$',
-                                               config.get('function')).groups()
-        if object_name:
-            module = __import__(module_name, fromlist=object_name)
-            object = getattr(module, object_name)
-            function = getattr(object, function_name)
-        else:
-            module = __import__(module_name, fromlist=function_name)
-            function = getattr(module, function_name)
+        function_path = config.get('function').split('.')
+        module = builtins
+        for i, key in enumerate(function_path[:-1]):
+            if not hasattr(module, key):
+                module_name = '.'.join(function_path[:i+1])
+                try:
+                    module = import_module(module_name)
+                except ModuleNotFoundError:
+                    raise ModuleNotFoundError(module_name)
+            else:
+                module = getattr(module, key)
+        function = getattr(module, function_path[-1])
 
         # Parse arguments
         args = config.get('args', [])
-        kwargs = config.get('kwargs', {})
-        req = config.get('requires', {})
-        req = {k: self.__getitem__(v) for k, v in req.items()}
+        for k, v in _items(args):
+            if isinstance(v, str):
+                if _is_string_argument(v):
+                    args[k] = _parse_string_argument(v)
+                else:
+                    # get variable or table column
+                    args[k] = self[v]
 
         # Call function
-        return function(*args, **kwargs, **req)
+        if isinstance(args, Mapping):
+            result = function(**args)
+        else:
+            result = function(*args)
+
+        return result
 
     def __getitem__(self, label):
         name, key = re.search(r'^(\w*)\.?(\w*)$', label).groups()


Test patch:
diff --git a/skypy/pipeline/tests/data/test_config.yml b/skypy/pipeline/tests/data/test_config.yml
index 1f7a612..0513bc1 100644
--- a/skypy/pipeline/tests/data/test_config.yml
+++ b/skypy/pipeline/tests/data/test_config.yml
@@ -1,33 +1,29 @@
 test_int: 1
 test_float: 1.0
+test_str: hello world
 test_cosmology:
-  module: astropy.cosmology
-  function: FlatLambdaCDM
-  kwargs:
+  function: astropy.cosmology.FlatLambdaCDM
+  args:
     H0: 67.74
     Om0: 0.3075
 tables:
   test_table_1:
     test_column_1:
-      module: numpy.random
-      function: uniform
-      kwargs:
+      function: numpy.random.uniform
+      args:
         size: 10
     test_column_2:
-      module: numpy.random
-      function: uniform
-      requires:
+      function: numpy.random.uniform
+      args:
         low: test_table_1.test_column_1
     test_column_3:
-      module: numpy.random
-      function: uniform
-      requires:
+      function: numpy.random.uniform
+      args:
         low: test_table_1.test_column_1
         high: test_table_1.test_column_2
   test_table_2:
     init:
-      module: astropy.table
-      function: QTable
-      kwargs:
+      function: astropy.table.QTable
+      args:
         names: ['column_1', 'column_2']
       depends: [test_table_1.complete]
diff --git a/skypy/pipeline/tests/test_driver.py b/skypy/pipeline/tests/test_driver.py
index 4231f08..d130857 100644
--- a/skypy/pipeline/tests/test_driver.py
+++ b/skypy/pipeline/tests/test_driver.py
@@ -12,8 +12,7 @@ def test_driver():
 
     # Evaluate and store the default astropy cosmology.
     config = {'test_cosmology': {
-                'module': 'astropy.cosmology',
-                'function': 'default_cosmology.get'}}
+                'function': 'astropy.cosmology.default_cosmology.get'}}
 
     driver = SkyPyDriver()
     driver.execute(config)
@@ -22,18 +21,21 @@ def test_driver():
     # Generate a simple two column table with a dependency. Also write the
     # table to a fits file and check it's contents.
     size = 100
+    string = '"' + size*'a' + '"'
     config = {'tables': {
                 'test_table': {
                   'column1': {
-                    'module': 'numpy.random',
-                    'function': 'uniform',
-                    'kwargs': {
+                    'function': 'numpy.random.uniform',
+                    'args': {
                       'size': size}},
                   'column2': {
-                    'module': 'numpy.random',
-                    'function': 'uniform',
-                    'requires': {
-                      'low': 'test_table.column1'}}}}}
+                    'function': 'numpy.random.uniform',
+                    'args': {
+                      'low': 'test_table.column1'}},
+                  'column3': {
+                      'function': 'list',
+                      'args': [
+                        string]}}}}
 
     driver = SkyPyDriver()
     driver.execute(config, file_format='fits')
@@ -49,23 +51,33 @@ def test_driver():
 
     # Check that the existing output files are modified if overwrite is True
     new_size = 2 * size
-    config['tables']['test_table']['column1']['kwargs']['size'] = new_size
+    new_string = '"' + new_size*'a' + '"'
+    config['tables']['test_table']['column1']['args']['size'] = new_size
+    config['tables']['test_table']['column3']['args'][0] = new_string
     driver = SkyPyDriver()
     driver.execute(config, file_format='fits', overwrite=True)
     with fits.open('test_table.fits') as hdu:
         assert len(hdu[1].data) == new_size
 
+    # Check for failure if 'column1' calls a nonexistant module
+    config['tables']['test_table']['column1']['function'] = 'nomodule.function'
+    with pytest.raises(ModuleNotFoundError):
+        SkyPyDriver().execute(config)
+
+    # Check for failure if 'column1' calls a nonexistant function
+    config['tables']['test_table']['column1']['function'] = 'builtins.nofunction'
+    with pytest.raises(AttributeError):
+        SkyPyDriver().execute(config)
+
     # Check for failure if 'column1' requires itself creating a cyclic
     # dependency graph
-    config['tables']['test_table']['column1']['requires'] = \
-        {'low': 'test_table.column1'}
+    config['tables']['test_table']['column1']['args'] = {'low': 'test_table.column1'}
     with pytest.raises(networkx.NetworkXUnfeasible):
         SkyPyDriver().execute(config)
 
     # Check for failure if 'column1' and 'column2' both require each other
     # creating a cyclic dependency graph
-    config['tables']['test_table']['column1']['requires'] = \
-        {'low': 'test_table.column2'}
+    config['tables']['test_table']['column1']['args'] = {'low': 'test_table.column2'}
     with pytest.raises(networkx.NetworkXUnfeasible):
         SkyPyDriver().execute(config)
 
@@ -77,13 +89,30 @@ def test_driver():
 
     # Check variables intialised by value
     config = {'test_int': 1,
-              'test_float': 1.0}
+              'test_float': 1.0,
+              'test_string': 'hello world'}
     driver = SkyPyDriver()
     driver.execute(config)
     assert isinstance(driver.test_int, int)
     assert isinstance(driver.test_float, float)
+    assert isinstance(driver.test_string, str)
     assert driver.test_int == 1
     assert driver.test_float == 1.0
+    assert driver.test_string == 'hello world'
+
+    # Check variables intialised by function
+    config = {'test_func': {
+                'function': 'list',
+                'args': [
+                    '"hello world"']},
+              'test_func2': {
+                'function': 'len',
+                'args': [
+                    'test_func']}}
+    driver = SkyPyDriver()
+    driver.execute(config)
+    assert driver.test_func == list('hello world')
+    assert driver.test_func2 == 11
 
 
 def teardown_module(module):