Fix bug in passing multiple set config per task via cli (#78)

Previously dict would be updated at the base-level meaning any parameters at
level below task would be overridden. Here use the executor set_config method
which already handles dict update at appropriate depth.

Resolves #77

Author: amitschang

dplutils/cli.py

@@ -1,6 +1,5 @@
 import json
 from argparse import ArgumentParser, Namespace
-from dplutils.pipeline.utils import dict_from_coord
 from dplutils.pipeline import PipelineExecutor
 
 
@@ -49,14 +48,6 @@ def parse_config_element(conf):
     return k, v
 
 
-def config_dict_from_args(args):
-    config = {}
-    for conf in args.set_config:
-        k,v = parse_config_element(conf)
-        config.update(dict_from_coord(k, v))
-    return config
-
-
 def set_config_from_args(pipeline: PipelineExecutor, args: Namespace):
     """Configure pipeline using config from arguments
 
@@ -72,9 +63,8 @@ def set_config_from_args(pipeline: PipelineExecutor, args: Namespace):
     """
     for ctx in args.set_context:
         pipeline.set_context(*parse_config_element(ctx))
-    config = config_dict_from_args(args)
-    if config:
-        pipeline.set_config_from_dict(config)
+    for conf in args.set_config:
+        pipeline.set_config(*parse_config_element(conf))
 
 
 def cli_run(pipeline: PipelineExecutor, args: Namespace|None = None,  **argparse_kwargs):

tests/test_cli.py

@@ -1,11 +1,13 @@
 import pytest
 from dplutils import cli
+from dplutils.pipeline import PipelineTask
 
 
 TEST_ARGV = [
     '',
     '--set-context', 'ctx=ctx-value',
     '--set-config', 'a.b.c=[1,2,3]',
+    '--set-config', 'a.b.x=99',
     '--set-config', 'x.y=1',
     '--set-config', 'l.m=string',
 ]
@@ -19,15 +21,7 @@ def sys_argv(monkeypatch):
 def test_get_argparser_adds_default_arguments(sys_argv):
     args = cli.get_argparser().parse_args()
     assert args.set_context == ['ctx=ctx-value']
-    assert args.set_config == ['a.b.c=[1,2,3]', 'x.y=1', 'l.m=string']
-
-
-def test_config_from_args(sys_argv):
-    args = cli.get_argparser().parse_args()
-    config = cli.config_dict_from_args(args)
-    assert config['a']['b']['c'] == [1, 2, 3]
-    assert config['x']['y'] == 1
-    assert config['l']['m'] == 'string'
+    assert args.set_config == ['a.b.c=[1,2,3]', 'a.b.x=99', 'x.y=1', 'l.m=string']
 
 
 def test_pipeline_set_config_from_args_no_args(monkeypatch, dummy_executor):
@@ -38,9 +32,24 @@ def test_pipeline_set_config_from_args_no_args(monkeypatch, dummy_executor):
 def test_run_with_cli_helper(monkeypatch, dummy_executor, tmp_path):
     assert len(list(tmp_path.glob('*.parquet'))) == 0
     monkeypatch.setattr(
-        'sys.argv',
-        ['', '-o', str(tmp_path), '--set-context', 'ctxvar=value', '--set-config', 'task1.num_cpus=2'])
+        'sys.argv', [
+            '',
+            '-o',
+            str(tmp_path),
+            '--set-context', 'ctxvar=value',
+            '--set-config', 'task1.num_cpus=2',
+            '--set-config', 'task1.batch_size=10',
+            '--set-config', 'task_kw.kwargs.a=[1,2]',
+            '--set-config', 'task_kw.kwargs.b=99',
+        ])
+    def task_with_kwargs(indf, a=None, b=None):
+        return indf
+    # patch in kwargs task to test kwarg setting via dotted notation
+    dummy_executor.graph.add_edge(dummy_executor.graph.task_map['task2'], PipelineTask('task_kw', task_with_kwargs))
     cli.cli_run(dummy_executor)
     assert dummy_executor.ctx['ctxvar'] == 'value'
     assert dummy_executor.tasks_idx['task1'].num_cpus == 2
+    assert dummy_executor.tasks_idx['task1'].batch_size == 10
+    assert dummy_executor.tasks_idx['task_kw'].kwargs['a'] == [1,2]
+    assert dummy_executor.tasks_idx['task_kw'].kwargs['b'] == 99
     assert len(list(tmp_path.glob('*.parquet'))) == 10