Repository: TileDB-Inc/TileDB-Py (python)

Issue:
[BUG] exception when call `label_index()` on a Query object
when call

```python
A.query(attrs=['name'], coords=True).label_index(['dt'])
```
has a exception below:

```shell
---------------------------------------------------------------------------
ModuleNotFoundError                       Traceback (most recent call last)
Cell In[94], [line 24](vscode-notebook-cell:?execution_count=94&line=24)
     [22](vscode-notebook-cell:?execution_count=94&line=22)     print(non_empty_domain)
     [23](vscode-notebook-cell:?execution_count=94&line=23)     print(A.meta['last_day'])
---> [24](vscode-notebook-cell:?execution_count=94&line=24)     print(A.query(attrs=['name'], coords=True).label_index(['dt', 'symbol']))
     [25](vscode-notebook-cell:?execution_count=94&line=25)     # print(A.query(attrs=['name'], coords=True).label_index(['dt', 'symbol'])[:])
     [26](vscode-notebook-cell:?execution_count=94&line=26)     # print(a.keys())
     [28](vscode-notebook-cell:?execution_count=94&line=28) schema = tiledb.ArraySchema.load(array_bars_stock_daily)

File libtiledb.pyx:2158, in tiledb.libtiledb.Query.label_index()

ModuleNotFoundError: No module named 'tiledb.multirange_indexer'
```

In the source file `libtiledb.pyx`

```python
    def label_index(self, labels):
        """Apply Array.label_index with query parameters."""
        from .multirange_indexer import LabelIndexer
        return LabelIndexer(self.array, tuple(labels), query=self)
```

`multirange_indexer.py` has already changed to `multirange_indexing.py`, so module 'tiledb.multirange_indexer' cannot be found.


Pull request description:
Fix exception when calling `label_index` on a Query object
Closes #2043 

Patch:
diff --git a/tiledb/libtiledb.pyx b/tiledb/libtiledb.pyx
index dd871839..0a20b112 100644
--- a/tiledb/libtiledb.pyx
+++ b/tiledb/libtiledb.pyx
@@ -2158,7 +2158,7 @@ cdef class Query(object):
 
     def label_index(self, labels):
         """Apply Array.label_index with query parameters."""
-        from .multirange_indexer import LabelIndexer
+        from .multirange_indexing import LabelIndexer
         return LabelIndexer(self.array, tuple(labels), query=self)
 
     @property
diff --git a/tiledb/query_condition.py b/tiledb/query_condition.py
index 5acb47e9..39e722f2 100644
--- a/tiledb/query_condition.py
+++ b/tiledb/query_condition.py
@@ -118,7 +118,7 @@ class QueryCondition:
         if not isinstance(self.c_obj, qc.PyQueryCondition):
             raise TileDBError(
                 "Malformed query condition statement. A query condition must "
-                "be made up of one or more Boolean expressions."
+                "be made up of one or more boolean expressions."
             )
 
 
@@ -164,9 +164,22 @@ class QueryConditionTree(ast.NodeVisitor):
     def visit_NotIn(self, node):
         return node
 
+    def visit_Is(self, node):
+        raise TileDBError("the `is` operator is not supported")
+
+    def visit_IsNot(self, node):
+        raise TileDBError("the `is not` operator is not supported")
+
     def visit_List(self, node):
         return list(node.elts)
 
+    def visit_Attribute(self, node) -> qc.PyQueryCondition:
+        raise TileDBError(
+            f"Unhandled dot operator in {ast.dump(node)} -- if your attribute name "
+            'has a dot in it, e.g. `orig.ident`, please wrap it with `attr("...")`, '
+            'e.g. `attr("orig.ident")`'
+        )
+
     def visit_Compare(self, node: Type[ast.Compare]) -> qc.PyQueryCondition:
         operator = self.visit(node.ops[0])
 
@@ -219,6 +232,9 @@ class QueryConditionTree(ast.NodeVisitor):
             op = qc.TILEDB_IN if isinstance(operator, ast.In) else qc.TILEDB_NOT_IN
             result = self.create_pyqc(dtype)(self.ctx, node.left.id, values, op)
 
+        else:
+            raise TileDBError(f"unrecognized operator in <<{ast.dump(node)}>>")
+
         return result
 
     def aux_visit_Compare(
@@ -344,20 +360,10 @@ class QueryConditionTree(ast.NodeVisitor):
 
         if isinstance(value_node, ast.Constant):
             value = value_node.value
-        elif isinstance(value_node, ast.Constant):
-            # deprecated in 3.8
-            value = value_node.value
-        elif isinstance(value_node, ast.Constant):
-            # deprecated in 3.8
-            value = value_node.n
-        elif isinstance(value_node, ast.Constant) or isinstance(
-            value_node, ast.Constant
-        ):
-            # deprecated in 3.8
-            value = value_node.s
         else:
             raise TileDBError(
-                f"Incorrect type for comparison value: {ast.dump(value_node)}"
+                f"Incorrect type for comparison value: {ast.dump(value_node)}: right-hand sides must be constant"
+                " expressions, not variables -- did you mean to quote the right-hand side as a string?"
             )
 
         return value
@@ -425,8 +431,12 @@ class QueryConditionTree(ast.NodeVisitor):
         result = self.visit(node.left)
         rhs = node.right[1:] if isinstance(node.right, list) else [node.right]
         for value in rhs:
-            result = result.combine(self.visit(value), op)
-
+            visited = self.visit(value)
+            if not isinstance(result, qc.PyQueryCondition):
+                raise Exception(
+                    f"Unable to parse expression component {ast.dump(node)} -- did you mean to quote it as a string?"
+                )
+            result = result.combine(visited, op)
         return result
 
     def visit_BoolOp(self, node: ast.BoolOp) -> qc.PyQueryCondition:


Test patch:
diff --git a/tiledb/tests/test_dimension_label.py b/tiledb/tests/test_dimension_label.py
index 0daeab2e..bb7c98e5 100644
--- a/tiledb/tests/test_dimension_label.py
+++ b/tiledb/tests/test_dimension_label.py
@@ -1,3 +1,5 @@
+from collections import OrderedDict
+
 import numpy as np
 import pytest
 
@@ -416,3 +418,68 @@ class DimensionLabelTestCase(DiskTestCase):
                             result["value"], attr_data[:, index:]
                         )
                     np.testing.assert_array_equal(result[label_name], label_index)
+
+    @pytest.mark.skipif(
+        tiledb.libtiledb.version()[0] == 2 and tiledb.libtiledb.version()[1] < 15,
+        reason="dimension labels requires libtiledb version 2.15 or greater",
+    )
+    def test_dimension_label_on_query(self):
+        uri = self.path("query_label_index")
+
+        dim1 = tiledb.Dim("d1", domain=(1, 4))
+        dim2 = tiledb.Dim("d2", domain=(1, 3))
+        dom = tiledb.Domain(dim1, dim2)
+        att = tiledb.Attr("a1", dtype=np.int64)
+        dim_labels = {
+            0: {"l1": dim1.create_label_schema("decreasing", np.int64)},
+            1: {
+                "l2": dim2.create_label_schema("increasing", np.int64),
+                "l3": dim2.create_label_schema("increasing", np.float64),
+            },
+        }
+        schema = tiledb.ArraySchema(domain=dom, attrs=(att,), dim_labels=dim_labels)
+        tiledb.Array.create(uri, schema)
+
+        a1_data = np.reshape(np.arange(1, 13), (4, 3))
+        l1_data = np.arange(4, 0, -1)
+        l2_data = np.arange(-1, 2)
+        l3_data = np.linspace(0, 1.0, 3)
+
+        with tiledb.open(uri, "w") as A:
+            A[:] = {"a1": a1_data, "l1": l1_data, "l2": l2_data, "l3": l3_data}
+
+        with tiledb.open(uri, "r") as A:
+            np.testing.assert_equal(
+                A.query().label_index(["l1"])[3:4],
+                OrderedDict(
+                    {"l1": np.array([4, 3]), "a1": np.array([[1, 2, 3], [4, 5, 6]])}
+                ),
+            )
+            np.testing.assert_equal(
+                A.query().label_index(["l1", "l3"])[2, 0.5:1.0],
+                OrderedDict(
+                    {
+                        "l3": np.array([0.5, 1.0]),
+                        "l1": np.array([2]),
+                        "a1": np.array([[8, 9]]),
+                    }
+                ),
+            )
+            np.testing.assert_equal(
+                A.query().label_index(["l2"])[:, -1:0],
+                OrderedDict(
+                    {
+                        "l2": np.array([-1, 0]),
+                        "a1": np.array([[1, 2], [4, 5], [7, 8], [10, 11]]),
+                    },
+                ),
+            )
+            np.testing.assert_equal(
+                A.query().label_index(["l3"])[:, 0.5:1.0],
+                OrderedDict(
+                    {
+                        "l3": np.array([0.5, 1.0]),
+                        "a1": np.array([[2, 3], [5, 6], [8, 9], [11, 12]]),
+                    },
+                ),
+            )
diff --git a/tiledb/tests/test_query_condition.py b/tiledb/tests/test_query_condition.py
index b47f69be..07549482 100644
--- a/tiledb/tests/test_query_condition.py
+++ b/tiledb/tests/test_query_condition.py
@@ -208,6 +208,13 @@ class QueryConditionTest(DiskTestCase):
 
     def test_string_sparse(self):
         with tiledb.open(self.create_input_array_UIDSA(sparse=True)) as A:
+            with self.assertRaises(tiledb.TileDBError) as exc_info:
+                A.query(cond="S == c", attrs=["S"])[:]
+            assert (
+                "right-hand sides must be constant expressions, not variables -- did you mean to quote the right-hand side as a string?"
+                in str(exc_info.value)
+            )
+
             result = A.query(cond="S == 'c'", attrs=["S"])[:]
             assert len(result["S"]) == 1
             assert result["S"][0] == b"c"
@@ -224,6 +231,13 @@ class QueryConditionTest(DiskTestCase):
 
     def test_string_dense(self):
         with tiledb.open(self.create_input_array_UIDSA(sparse=False)) as A:
+            with self.assertRaises(tiledb.TileDBError) as exc_info:
+                A.query(cond="S == ccc", attrs=["S"])[:]
+            assert (
+                "right-hand sides must be constant expressions, not variables -- did you mean to quote the right-hand side as a string?"
+                in str(exc_info.value)
+            )
+
             result = A.query(cond="S == 'ccc'", attrs=["S"])[:]
             assert all(self.filter_dense(result["S"], A.attr("S").fill) == b"c")
 
@@ -594,6 +608,24 @@ class QueryConditionTest(DiskTestCase):
                 exc_info.value
             )
 
+    @pytest.mark.parametrize(
+        "expression_and_message",
+        [
+            ["foo is True", "the `is` operator is not supported"],
+            ["foo is not True", "the `is not` operator is not supported"],
+            [
+                "foo &&& bar",
+                "Could not parse the given QueryCondition statement: foo &&& bar",
+            ],
+        ],
+    )
+    @pytest.mark.parametrize("sparse", [True, False])
+    def test_not_supported_operators(self, expression_and_message, sparse):
+        with tiledb.open(self.create_input_array_UIDSA(sparse=sparse)) as A:
+            expression, message = expression_and_message
+            with self.assertRaisesRegex(tiledb.TileDBError, message):
+                A.query(cond=expression)[:]
+
     @pytest.mark.skipif(not has_pandas(), reason="pandas>=1.0,<3.0 not installed")
     def test_dense_datetime(self):
         import pandas as pd
@@ -753,6 +785,37 @@ class QueryConditionTest(DiskTestCase):
             # ensures that ' a' does not match 'a'
             assert len(result["A"]) == 0
 
+    def test_attribute_with_dot(self):
+        path = self.path("test_with_dot")
+        dom = tiledb.Domain(tiledb.Dim(name="dim", domain=(0, 10), dtype=np.uint32))
+        attrs = [
+            tiledb.Attr(name="attr.one", dtype=np.uint32),
+            tiledb.Attr(name="attr.two", dtype=np.uint32),
+        ]
+        schema = tiledb.ArraySchema(domain=dom, attrs=attrs, sparse=True)
+        tiledb.Array.create(path, schema)
+        with tiledb.open(path, "w") as A:
+            A[np.arange(11)] = {"attr.one": np.arange(11), "attr.two": np.arange(11)}
+        with tiledb.open(path, "r") as A:
+            with pytest.raises(tiledb.TileDBError) as exc_info:
+                A.query(cond="attr.one < 6")[:]
+            assert (
+                "TileDBError: Unhandled dot operator in Attribute(value=Name(id='attr', ctx=Load()), attr='one', ctx=Load()) -- if your attribute name has a dot in it, e.g. `orig.ident`, please wrap it with `attr(\"...\")`, e.g. `attr(\"orig.ident\")`"
+                in str(exc_info.value)
+            )
+            with pytest.raises(tiledb.TileDBError) as exc_info:
+                A.query(cond="attr.two >= 6")[:]
+            assert (
+                "TileDBError: Unhandled dot operator in Attribute(value=Name(id='attr', ctx=Load()), attr='two', ctx=Load()) -- if your attribute name has a dot in it, e.g. `orig.ident`, please wrap it with `attr(\"...\")`, e.g. `attr(\"orig.ident\")`"
+                in str(exc_info.value)
+            )
+
+            # now test with the correct syntax
+            result = A.query(cond='attr("attr.one") < 6')[:]
+            assert_array_equal(result["attr.one"], A[:6]["attr.one"])
+            result = A.query(cond='attr("attr.two") >= 6')[:]
+            assert_array_equal(result["attr.two"], A[6:]["attr.two"])
+
     @pytest.mark.skipif(not has_pandas(), reason="pandas>=1.0,<3.0 not installed")
     def test_do_not_return_attrs(self):
         with tiledb.open(self.create_input_array_UIDSA(sparse=True)) as A: