Repository: Azure/pykusto

Issue:
In some contexts a dynamic literal needs to be created with the "dynamic" keyword
For example, currently `strcat_array` fails because it requires:
`strcat_array(dynamic(["A", "B", "C"]), ",")`
while pykusto produces:
`strcat_array(("A", "B", "C"), ",")`

Discussion:

Patch:
diff --git a/pykusto/expressions.py b/pykusto/expressions.py
index a02183b..ec73163 100644
--- a/pykusto/expressions.py
+++ b/pykusto/expressions.py
@@ -21,14 +21,6 @@ OrderType = Union[DatetimeType, TimespanType, NumberType, StringType]
 # All classes in the same file to prevent circular dependencies
 
 def _subexpr_to_kql(obj: ExpressionType) -> KQL:
-    """
-    Convert the given expression to KQL, enclosing it in parentheses if it is a compound expression. This guarantees
-    correct evaluation order. When parentheses are not needed, for example when the expressions is used as an argument
-    to a function, use `to_kql` instead.
-
-    :param obj: Expression to convert to KQL
-    :return: KQL that represents the given expression
-    """
     if isinstance(obj, BaseExpression):
         return obj.as_subexpression()
     return to_kql(obj)
@@ -42,11 +34,8 @@ class BaseExpression:
             raise TypeError("BaseExpression is abstract")
         return object.__new__(cls)
 
-    def __init__(self, kql: Union[KQL, 'BaseExpression']) -> None:
-        if isinstance(kql, BaseExpression):
-            self.kql = kql.kql
-        elif not isinstance(kql, str):
-            raise ValueError("Either expression or KQL required")
+    def __init__(self, kql: KQL) -> None:
+        assert isinstance(kql, str)
         self.kql = kql
 
     def __repr__(self) -> str:
@@ -270,9 +259,9 @@ class StringExpression(BaseExpression):
 
     def split(self, delimiter: StringType, requested_index: NumberType = None) -> 'ArrayExpression':
         if requested_index is None:
-            return ArrayExpression(KQL('split({}, {})'.format(to_kql(self.kql), to_kql(delimiter))))
-        return ArrayExpression(KQL('split({}, {}, {})'.format(
-            to_kql(self.kql), to_kql(delimiter), to_kql(requested_index)
+            return ArrayExpression(KQL('split({}, {}'.format(self.kql, to_kql(delimiter))))
+        return ArrayExpression(KQL('split({}, {}, {}'.format(
+            self.kql, _subexpr_to_kql(delimiter), to_kql(requested_index)
         )))
 
     def equals(self, other: StringType, case_sensitive: bool = False) -> BooleanExpression:
@@ -654,15 +643,6 @@ column_generator = ColumnGenerator()
 
 
 def to_kql(obj: ExpressionType) -> KQL:
-    """
-    Convert the given expression to KQL. If this is a subexpression of a greater expression, neighboring operators might
-    take precedence over operators included in this expression, causing an incorrect evaluation order.
-    If this is a concern, use `_subexpr_to_kql` instead, which will enclose this expression in parentheses if it is
-    a compound expression.
-
-    :param obj: Expression to convert to KQL
-    :return: KQL that represents the given expression
-    """
     if isinstance(obj, BaseExpression):
         return obj.kql
     return kql_converter.for_obj(obj)
diff --git a/pykusto/functions.py b/pykusto/functions.py
index 6269870..fcfc6f4 100644
--- a/pykusto/functions.py
+++ b/pykusto/functions.py
@@ -1,10 +1,10 @@
+import json
 from typing import Union
 
 from pykusto.expressions import Column, NumberType, NumberExpression, TimespanType, \
     DatetimeExpression, TimespanExpression, ArrayType, DynamicType, DatetimeType, BaseExpression, BooleanType, \
     ExpressionType, AggregationExpression, StringType, StringExpression, BooleanExpression, \
-    NumberAggregationExpression, MappingAggregationExpression, ArrayAggregationExpression, to_kql, DynamicExpression, \
-    ArrayExpression
+    NumberAggregationExpression, MappingAggregationExpression, ArrayAggregationExpression, to_kql, DynamicExpression
 from pykusto.kql_converters import KQL
 from pykusto.type_utils import plain_expression
 
@@ -559,8 +559,7 @@ def sign(expr: NumberType) -> NumberExpression:
 # def sin(self): return
 #
 #
-def split(string: StringType, delimiter: StringType, requested_index: NumberType = None) -> 'ArrayExpression':
-    return StringExpression(KQL(string)).split(delimiter, requested_index)
+# def split(self): return
 
 
 def sqrt(expr: NumberType) -> NumberExpression:
@@ -583,16 +582,20 @@ def startofyear(expr: DatetimeType, offset: NumberType = None) -> DatetimeExpres
     return expr.startofyear(offset)
 
 
-def strcat(expr1: StringType, expr2: StringType, *exprs: StringType) -> StringExpression:
-    res = 'strcat({}, {}'.format(to_kql(expr1),
-                                 to_kql(expr2))
-    if len(exprs) > 0:
-        res = res + ', ' + ', '.join([to_kql(expr) for expr in exprs])
-    return StringExpression(KQL(res + ')'))
+def strcat(*strings: StringType) -> StringExpression:
+    if len(strings) < 2:
+        raise ValueError("strcat requires at least two arguments")
+    return StringExpression(KQL('strcat({})'.format(', '.join(to_kql(s) for s in strings))))
+
+
+def to_literal_dynamic(d: DynamicType) -> KQL:
+    if isinstance(d, BaseExpression):
+        return d.kql
+    return KQL('dynamic({})'.format(json.dumps(d)))
 
 
 def strcat_array(expr: ArrayType, delimiter: StringType) -> StringExpression:
-    return StringExpression(KQL('strcat_array({}, {})'.format(to_kql(expr), to_kql(delimiter))))
+    return StringExpression(KQL('strcat_array({}, {})'.format(to_literal_dynamic(expr), to_kql(delimiter))))
 
 
 def strcat_delim(delimiter: StringType, expr1: StringType, expr2: StringType, *exprs: StringType) -> StringExpression:
diff --git a/pykusto/kql_converters.py b/pykusto/kql_converters.py
index 63079f4..a1eaeae 100644
--- a/pykusto/kql_converters.py
+++ b/pykusto/kql_converters.py
@@ -29,7 +29,7 @@ def timedelta_to_kql(td: timedelta) -> KQL:
 @kql_converter(Mapping, List, Tuple)
 def dynamic_to_kql(d: Union[Mapping, List, Tuple]) -> KQL:
     query = list(json.dumps(d))
-    # Issue #11
+    # Convert square brackets to round brackets (Issue #11)
     counter = 0
     prev = ""
     for i, c in enumerate(query):
diff --git a/pykusto/logger.py b/pykusto/logger.py
deleted file mode 100644
index 6b2cc66..0000000
--- a/pykusto/logger.py
+++ /dev/null
@@ -1,3 +0,0 @@
-import logging
-
-logger = logging.getLogger("pykusto")
diff --git a/pykusto/query.py b/pykusto/query.py
index 845517c..8207f13 100644
--- a/pykusto/query.py
+++ b/pykusto/query.py
@@ -1,18 +1,18 @@
-from abc import abstractmethod
-from copy import copy, deepcopy
-from enum import Enum
 from itertools import chain
-from types import FunctionType
 from typing import Tuple, List, Union, Optional
 
+from abc import abstractmethod
 from azure.kusto.data.helpers import dataframe_from_result_table
+from copy import copy, deepcopy
+from enum import Enum
+from types import FunctionType
 
 from pykusto.client import Table
 from pykusto.expressions import BooleanType, ExpressionType, AggregationExpression, OrderType, \
     StringType, AssignmentBase, AssignmentFromAggregationToColumn, AssignmentToSingleColumn, Column, BaseExpression, \
     AssignmentFromColumnToColumn, AnyExpression, to_kql
 from pykusto.kql_converters import KQL
-from pykusto.logger import logger
+from pykusto.type_utils import logger
 from pykusto.udf import stringify_python_func
 
 
diff --git a/pykusto/type_utils.py b/pykusto/type_utils.py
index ca407cd..bb5cb4d 100644
--- a/pykusto/type_utils.py
+++ b/pykusto/type_utils.py
@@ -1,65 +1,41 @@
+import logging
 from datetime import datetime, timedelta
 from numbers import Number
-from typing import Union, Mapping, Type, Dict, Callable, Tuple, List
+from typing import Union, Mapping, Type, Dict, Callable, Any, Tuple, List
+
+logger = logging.getLogger("pykusto")
 
 KustoTypes = Union[str, Number, bool, datetime, Mapping, List, Tuple, timedelta]
 # TODO: Unhandled data types: guid, decimal
 
 
 class TypeRegistrar:
-    """
-    A factory for annotations that are used to create a mapping between Kusto types and python types and functions.
-    Each annotation must be called with a Kusto type as a parameter. The `for_obj` and `for_type` methods
-    can then be used to retrieve the python type or function corresponding to a given Kusto type.
-    """
     registry: Dict[Type[KustoTypes], Callable]
 
-    def __init__(self, name: str) -> None:
-        """
-        :param name: Name is used for better logging and clearer errors
-        """
-        self.name = name
+    def __init__(self) -> None:
         self.registry = {}
 
-    def __repr__(self) -> str:
-        return self.name
-
     def __call__(self, *types: Type[KustoTypes]) -> Callable:
         def inner(wrapped):
             for t in types:
-                previous = self.registry.setdefault(t, wrapped)
-                if previous is not wrapped:
-                    raise TypeError("{}: type already registered: {}".format(self, t.__name__))
+                self.registry[t] = wrapped
             return wrapped
 
         return inner
 
-    def for_obj(self, obj: KustoTypes) -> Callable:
-        """
-        Given an object of Kusto type, retrieve the python type or function associated with the object's type, and call
-        it with the given object as a parameter
-
-        :param obj: An object of Kusto type
-        :return: Associated python object
-        """
+    def for_obj(self, obj: Any) -> Any:
         for registered_type, registered_callable in self.registry.items():
             if isinstance(obj, registered_type):
                 return registered_callable(obj)
-        raise ValueError("{}: no registered callable for object {} of type {}".format(self, obj, type(obj).__name__))
+        raise ValueError("No registered callable for object {} of type {}".format(obj, type(obj).__name__))
 
     def for_type(self, t: Type[KustoTypes]) -> Callable:
-        """
-        Given a Kusto type, retrieve the associated python type or function
-
-        :param t: A Kusto type
-        :return: Associated python object
-        """
         for registered_type, registered_callable in self.registry.items():
             if issubclass(t, registered_type):
                 return registered_callable
-        raise ValueError("{}: no registered callable for type {}".format(self, t.__name__))
+        raise ValueError("No registered callable for type {}".format(t.__name__))
 
 
-kql_converter = TypeRegistrar("KQL Converter")
-plain_expression = TypeRegistrar("Plain expression")
-aggregation_expression = TypeRegistrar("Aggregation expression")
+kql_converter = TypeRegistrar()
+plain_expression = TypeRegistrar()
+aggregation_expression = TypeRegistrar()
diff --git a/setup.py b/setup.py
index ca8f191..543081f 100644
--- a/setup.py
+++ b/setup.py
@@ -2,7 +2,7 @@ from setuptools import setup, find_packages
 
 setup(
     name='pykusto',
-    version='0.0.9',
+    version='0.0.7',
     packages=find_packages(exclude=['test']),
     url='https://github.com/Azure/pykusto',
     license='MIT License',


Test patch:
diff --git a/test/test_base.py b/test/test_base.py
index dde3185..6321ec8 100644
--- a/test/test_base.py
+++ b/test/test_base.py
@@ -3,7 +3,7 @@ import sys
 from typing import Callable
 from unittest import TestCase
 
-from pykusto.logger import logger
+from pykusto.type_utils import logger
 
 
 class TestBase(TestCase):
diff --git a/test/test_functions.py b/test/test_functions.py
index 3ce0a5c..65d0d07 100644
--- a/test/test_functions.py
+++ b/test/test_functions.py
@@ -1,5 +1,4 @@
 import datetime
-import unittest
 
 from pykusto import functions as f
 from pykusto.expressions import column_generator as col
@@ -372,7 +371,6 @@ class TestFunction(TestBase):
             Query().extend(f.strcat_delim('-', ',', col.foo)).render()
         )
 
-    @unittest.skip("Enabled after #40 is fixed")
     def test_strcat_array(self):
         self.assertEqual(
             " | where (strcat_array(foo, \",\")) == \"A,B,C\"",
@@ -425,20 +423,6 @@ class TestFunction(TestBase):
             Query().where(f.substring(col.foo, col.bar, 4) == 'ABC,ABC').render()
         )
 
-    def test_split(self):
-        self.assertEqual(
-            ' | extend foo = (split(bar, "_", 3))',
-            Query().extend(foo=f.split(col.bar, "_", 3)).render()
-        )
-        self.assertEqual(
-            ' | extend foo = (split(bar, "_")[3])',
-            Query().extend(foo=f.split(col.bar, "_")[3]).render()
-        )
-        self.assertEqual(
-            ' | extend foo = (split("1_2", "_")[3])',
-            Query().extend(foo=f.split("1_2", "_")[3]).render()
-        )
-
     def test_tobool(self):
         self.assertEqual(
             " | where tobool(foo)",
diff --git a/test/test_utils.py b/test/test_utils.py
index 2b2e7aa..fc774c4 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -1,81 +1,16 @@
 from pykusto.expressions import to_kql
-from pykusto.type_utils import TypeRegistrar
 from test.test_base import TestBase
 
 
 class TestUtils(TestBase):
     def test_dynamic_to_kql(self):
-        test_dict = {
+        dict ={
             "name": "Alan",
             "age": 21,
             "address": ("NY", 36),
             "pets": ["Libby", "Panda", "]", "["]
         }
         self.assertEqual(
-            "{\"name\": \"Alan\", \"age\": 21, \"address\": (\"NY\", 36), "
-            "\"pets\": (\"Libby\", \"Panda\", \"]\", \"[\")}",
-            to_kql(test_dict)
-        )
-
-    def test_type_registrar_for_type(self):
-        test_annotation = TypeRegistrar("Test annotation")
-
-        @test_annotation(str)
-        def str_annotated(s: str) -> str:
-            return "response to " + s
-
-        self.assertEqual(
-            "response to test for_type",
-            test_annotation.for_type(str)("test for_type")
-        )
-
-    def test_type_registrar_for_obj(self):
-        test_annotation = TypeRegistrar("Test annotation")
-
-        @test_annotation(str)
-        def str_annotated(s: str) -> str:
-            return "response to " + s
-
-        self.assertEqual(
-            "response to test for_obj",
-            test_annotation.for_obj("test for_obj")
-        )
-
-    def test_type_registrar_for_type_not_found(self):
-        test_annotation = TypeRegistrar("Test annotation")
-
-        @test_annotation(str)
-        def str_annotated(s: str) -> str:
-            return "response to " + s
-
-        self.assertRaises(
-            ValueError("Test annotation: no registered callable for type bool"),
-            lambda: test_annotation.for_type(bool)("test for_type")
-        )
-
-    def test_type_registrar_for_obj_not_found(self):
-        test_annotation = TypeRegistrar("Test annotation")
-
-        @test_annotation(str)
-        def str_annotated(s: str) -> str:
-            return "response to " + s
-
-        self.assertRaises(
-            ValueError("Test annotation: no registered callable for object True of type bool"),
-            lambda: test_annotation.for_obj(True)
-        )
-
-    def test_type_registrar_collision(self):
-        test_annotation = TypeRegistrar("Test annotation")
-
-        @test_annotation(str)
-        def str_annotated_1(s: str) -> str:
-            return "response to " + s
-
-        def str_annotated_2(s: str) -> str:
-            return "response to " + s
-
-        self.assertRaises(
-            TypeError("Test annotation: type already registered: str"),
-            lambda: test_annotation(str)(str_annotated_2)
+            "{\"name\": \"Alan\", \"age\": 21, \"address\": (\"NY\", 36), \"pets\": (\"Libby\", \"Panda\", \"]\", \"[\")}",
+            to_kql(dict)
         )