Fix TreeRoot interface to accept single ActionArgsPack argument

idle-code · idle-code · commit 17cbeab56552 · 2018-06-14T19:56:56.000+02:00
Add to_python_name and from_python_name methods to NodePath
diff --git a/contextshell/Action.py b/contextshell/Action.py
@@ -1,5 +1,6 @@
 from abc import ABC, abstractmethod
 from contextshell.NodePath import NodePath
+from contextshell.TreeRoot import ActionArgsPack
 from typing import Dict, Union, Any
 
 
@@ -10,5 +11,5 @@ def __init__(self, name: NodePath):
         self.name: NodePath = name
 
     @abstractmethod
-    def invoke(self, target: NodePath, action: NodePath, arguments: Dict[Union[NodePath, int], Any]):
+    def invoke(self, target: NodePath, action: NodePath, arguments: ActionArgsPack):
         raise NotImplementedError()
diff --git a/contextshell/CallableAction.py b/contextshell/CallableAction.py
@@ -1,6 +1,6 @@
 from collections import OrderedDict
 from typing import Callable, Dict, Union, Any, Tuple, List
-
+from contextshell.TreeRoot import unpack_argument_tree, ActionArgsPack
 from contextshell.Action import Action
 from contextshell.NodePath import NodePath
 
@@ -10,22 +10,11 @@ def __init__(self, implementation: Callable, name: NodePath):
         super().__init__(name)
         self.implementation = implementation
 
-    def invoke(self, target: NodePath, action: NodePath, arguments: Dict[Union[NodePath, int], Any]):
+    def invoke(self, target: NodePath, action: NodePath, arguments: ActionArgsPack):
         #print("Invoked with:", *args)
-        args, kwargs = CallableAction.unpack_argument_tree(arguments)
+        args, kwargs = unpack_argument_tree(arguments)
         return self.implementation(target, *args, **kwargs)
 
-    @staticmethod
-    def unpack_argument_tree(arguments: Dict[Union[NodePath, int], Any]) -> Tuple[List[Any], Dict[NodePath, Any]]:
-        args = list()
-        kwargs = OrderedDict()
-        for key, value in arguments.items():
-            if isinstance(key, int):
-                args.append(value)
-            else:
-                kwargs[key] = value
-        return args, kwargs
-
 
 def action_from_function(method_to_wrap: Callable) -> Action:
     action_name: str = method_to_wrap.__name__
diff --git a/contextshell/CommandInterpreter.py b/contextshell/CommandInterpreter.py
@@ -1,6 +1,7 @@
 from contextshell.Command import Command
 from contextshell.NodePath import NodePath
-from contextshell.TreeRoot import TreeRoot
+from contextshell.TreeRoot import TreeRoot, parse_argument_tree
+from typing import Dict, Union, Any
 
 
 class CommandInterpreter:
@@ -15,9 +16,9 @@ def execute(self, command: Command):
             raise RuntimeError("No action target specified")
         target_path = NodePath.cast(target_path)
         action_path = NodePath.cast(self._evaluate(command.name))
-        arguments = map(self._evaluate, command.arguments)
-        # TODO: parse arguments into OrderedDict[Union[int, NodePath], Any] as in Action.invoke interface
-        return self.tree.execute(target_path, action_path, *arguments)
+        arguments = list(map(self._evaluate, command.arguments))
+        arguments = parse_argument_tree(arguments)
+        return self.tree.execute(target_path, action_path, arguments)
 
     def _evaluate(self, part):
         if isinstance(part, Command):
diff --git a/contextshell/CommandParser.py b/contextshell/CommandParser.py
@@ -13,6 +13,7 @@ def convert_token_type(token):
         pass
     return token
 
+
 def tokenize(text: str) -> List[str]:
     tokens = []
     tok = ''
diff --git a/contextshell/NodePath.py b/contextshell/NodePath.py
@@ -1,24 +1,36 @@
 
+# TODO: add from_python_name and to_python_name met
+
 class NodePath(list):
     # CHECK: if this class could be replaced by build-in os.path
     separator = '.'
 
     @staticmethod
-    def join(first, *rest):
+    def join(first, *rest) -> 'NodePath':
         path = NodePath(first)
         for name in rest:
             part = NodePath.cast(name)
             path.extend(part)
         return path
 
     @staticmethod
-    def cast(path):
+    def cast(path) -> 'NodePath':
         """Converts passed argument to NodePath (if needed)"""
         # TODO: remove this method - be aware what types are passed instead
         if path is None:
             return NodePath()
         return NodePath(path)
 
+    @staticmethod
+    def from_python_name(name: str) -> 'NodePath':
+        name = name.lstrip('_').replace('_', NodePath.separator)
+        return NodePath.cast(name)
+
+    def to_python_name(self) -> str:
+        name = str(self)
+        name = name.lstrip(NodePath.separator).replace(NodePath.separator, '_')
+        return name
+
     def __init__(self, representation=[], absolute=False):
         super().__init__()
         self.is_absolute = absolute
diff --git a/contextshell/NodeTreeRoot.py b/contextshell/NodeTreeRoot.py
@@ -1,6 +1,6 @@
 from contextshell.Node import Node
 from contextshell.NodePath import NodePath
-from contextshell.TreeRoot import TreeRoot
+from contextshell.TreeRoot import TreeRoot, ActionArgsPack
 from contextshell.Action import Action
 from contextshell.CallableAction import action_from_function
 from typing import Callable, List, Optional, Dict, Union, Any
@@ -105,14 +105,14 @@ def is_action(self, path: NodePath):
         node_value = self.get(path)
         return self._is_action_implementation(node_value)
 
-    def execute(self, target: NodePath, action: NodePath, *args):
+    def execute(self, target: NodePath, action: NodePath, args: ActionArgsPack):
         #print("Execute: {}: {} {}".format(target, action, args))
         action_impl = self.find_action(target, action)
         if action_impl is None:
             raise NameError("Could not find action named '{}'".format(action))
-        from collections import OrderedDict  # FIXME: Temporary, until execute interface changes
-        arguments = OrderedDict(enumerate(args))
-        return action_impl.invoke(target, action, arguments)
+        #from collections import OrderedDict  # FIXME: Temporary, until execute interface changes
+        #arguments = OrderedDict(enumerate(args))
+        return action_impl.invoke(target, action, args)
 
     def create_node(self, value):
         return Node(value)
diff --git a/contextshell/TreeRoot.py b/contextshell/TreeRoot.py
@@ -1,8 +1,50 @@
 from abc import ABC, abstractmethod
 from contextshell.NodePath import NodePath
+from typing import Dict, Union, Any, Tuple, List
+from collections import OrderedDict
+
+
+ActionArgsPack = Dict[Union[NodePath, int], Any]
 
 
 class TreeRoot(ABC):
     @abstractmethod
-    def execute(self, target: NodePath, action: NodePath, *args):  #FIXME: use Dict[Union[NodePath, int], Any] as arguments
+    def execute(self, target: NodePath, action: NodePath, args: ActionArgsPack):
         raise NotImplementedError()
+
+def unpack_argument_tree(action_args: ActionArgsPack) -> Tuple[List[Any], Dict[str, Any]]:
+    args = dict()
+    kwargs = OrderedDict()
+    for key, value in action_args.items():
+        if isinstance(key, int):
+            args[key] = value
+        else:
+            kwargs[key.to_python_name()] = value
+    assert len(args) == 0 or max(args.keys()) < len(args)+len(kwargs)
+    args = [a[1] for a in sorted(args.items())]
+    return args, kwargs
+
+
+def pack_argument_tree(args: List[Any], kwargs: Dict[str, Any]) -> ActionArgsPack:
+    pack_list = []
+    for i, arg in enumerate(args):
+        pack_list.append((i, arg))
+    for key, value in kwargs.items():
+        pack_list.append((NodePath.from_python_name(key), value))
+    return OrderedDict(pack_list)
+
+
+def parse_argument_tree(raw_arguments: List[str]) -> ActionArgsPack:
+    from contextshell.CommandParser import convert_token_type
+    pack_list = []
+    for i, arg in enumerate(raw_arguments):
+        if isinstance(arg, str) and '=' in arg:
+            key, value = arg.split('=')
+            key_path = NodePath.from_python_name(key)
+            if key_path.is_absolute:
+                raise ValueError("Named argument path must be relative - {}".format(key_path))
+            typed_value = convert_token_type(value)
+            pack_list.append((key_path, typed_value))
+        else:
+            pack_list.append((i, arg))
+    return OrderedDict(pack_list)
diff --git a/tests/unit/ArgumentPackerTests.py b/tests/unit/ArgumentPackerTests.py
@@ -0,0 +1,161 @@
+import unittest
+from contextshell.TreeRoot import pack_argument_tree, unpack_argument_tree, parse_argument_tree
+from contextshell.NodePath import NodePath as np
+from collections import OrderedDict
+
+
+class UnpackArgumentTreeTests(unittest.TestCase):
+    def test_unpack_ordered(self):
+        packed = OrderedDict([
+            (0, 'foo'),
+            (1, 'bar'),
+        ])
+
+        args, _ = unpack_argument_tree(packed)
+
+        self.assertSequenceEqual(['foo', 'bar'], args)
+
+    def test_unpack_mixed_order(self):
+        packed = OrderedDict([
+            (1, 'bar'),
+            (0, 'foo'),
+        ])
+
+        args, _ = unpack_argument_tree(packed)
+
+        self.assertSequenceEqual(['foo', 'bar'], args)
+
+    def test_unpack_invalid_index(self):
+        packed = OrderedDict([
+            (1, 'bar'),
+        ])
+
+        with self.assertRaises(AssertionError):
+            unpack_argument_tree(packed)
+
+    def test_unpack_keyed(self):
+        packed = OrderedDict([
+            (np('foo'), 1),
+            (np('bar'), 2),
+        ])
+
+        _, kwargs = unpack_argument_tree(packed)
+
+        self.assertDictEqual({
+            np('foo'): 1,
+            np('bar'): 2,
+        }, kwargs)
+
+    def test_unpack_mixed(self):
+        packed = OrderedDict([
+            (np('foo'), 1),
+            (1, 'bar'),
+        ])
+
+        args, kwargs = unpack_argument_tree(packed)
+
+        self.assertSequenceEqual(['bar'], args)
+        self.assertDictEqual({
+            np('foo'): 1,
+        }, kwargs)
+
+    def test_unpack_path_to_python_name(self):
+        packed = OrderedDict([
+            (np('foo.bar'), 1)
+        ])
+
+        _, kwargs = unpack_argument_tree(packed)
+        self.assertDictEqual({
+            'foo_bar': 1,
+        }, kwargs)
+
+
+class PackArgumentTreeTests(unittest.TestCase):
+    def test_pack_ordered(self):
+        args = ['foo', 'bar']
+        kwargs = {}
+
+        packed = pack_argument_tree(args, kwargs)
+
+        self.assertEqual(OrderedDict([
+            (0, 'foo'),
+            (1, 'bar')
+        ]), packed)
+
+    def test_pack_keyed(self):
+        args = []
+        kwargs = {
+            'foo': 2,
+            'bar': 1
+        }
+
+        packed = pack_argument_tree(args, kwargs)
+
+        self.assertDictEqual({
+            np('bar'): 1,
+            np('foo'): 2
+        }, packed)
+
+    def test_key_is_converted_to_path(self):
+        args = []
+        kwargs = {
+            'foo_bar': 1,
+        }
+
+        packed = pack_argument_tree(args, kwargs)
+
+        self.assertDictEqual({
+            np('foo.bar'): 1
+        }, packed)
+
+
+class ParseArgumentTreeTests(unittest.TestCase):
+    def test_parse_empty(self):
+        args = []
+
+        packed = parse_argument_tree(args)
+
+        self.assertEqual(0, len(packed))
+
+    def test_parse_positional(self):
+        args = ['foo', 'bar']
+
+        packed = parse_argument_tree(args)
+
+        self.assertEqual(OrderedDict([
+            (0, 'foo'),
+            (1, 'bar'),
+        ]), packed)
+
+    def test_parse_keyword_string(self):
+        args = ['foo=rabarbar']
+
+        packed = parse_argument_tree(args)
+
+        self.assertEqual(OrderedDict([
+            (np('foo'), 'rabarbar'),
+        ]), packed)
+
+    def test_parse_keyword_int(self):
+        args = ['foo=3']
+
+        packed = parse_argument_tree(args)
+
+        self.assertEqual(OrderedDict([
+            (np('foo'), 3),
+        ]), packed)
+
+    def test_parse_keyword_path(self):
+        args = ['foo.bar=spam']
+
+        packed = parse_argument_tree(args)
+
+        self.assertEqual(OrderedDict([
+            (np('foo.bar'), 'spam'),
+        ]), packed)
+
+    def test_parse_absolute_path_raises(self):
+        args = ['.foo=spam']
+
+        with self.assertRaises(ValueError):
+            parse_argument_tree(args)
diff --git a/tests/unit/CallableActionTests.py b/tests/unit/CallableActionTests.py
@@ -34,8 +34,8 @@ def test_arguments_are_unpacked_to_keywords(self):
         implementation = Mock()
         action = create_action(implementation)
         arguments = OrderedDict([
-            ('foo', 'spam'),
-            ('bar', 2),
+            (np('foo'), 'spam'),
+            (np('bar'), 2),
             ])
 
         action.invoke(np('.target'), np('action'), arguments)
diff --git a/tests/unit/CommandInterpreterTests.py b/tests/unit/CommandInterpreterTests.py
diff --git a/tests/unit/Fakes.py b/tests/unit/Fakes.py
diff --git a/tests/unit/NodePathTests.py b/tests/unit/NodePathTests.py
