Fix parsing of $not expression on command line (#970)

* Add missing expression case and tests of signac find for all filters

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Address code review

* Use fixture for find_filter

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix pydocstyle

* Update changelog

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

changelog.txt

@@ -22,6 +22,11 @@ Changed
  - linked views now can contain spaces and other characters except directory separators (#926).
  - linked views now can be created on Windows, if 'Developer mode' is enabled (#430).
 
+Fixed
++++++
+
+ - Fixed parsing of ``$not`` query expressions on command line (#970).
+
 [2.1.0] -- 2023-07-12
 ---------------------
 

signac/filterparse.py

@@ -195,6 +195,7 @@ def parse_filter_arg(args):
 
 def _add_prefix(filter):
     """Add prefix "sp." to a (possibly nested) filter."""
+    # Logical operators ($and, $or, $not) should not be prefixed, but their values should.
     for key, value in filter.items():
         if key in ("$and", "$or"):
             if isinstance(value, list) or isinstance(value, tuple):
@@ -203,6 +204,8 @@ def _add_prefix(filter):
                 raise ValueError(
                     "The argument to a logical operator must be a list or a tuple!"
                 )
+        elif key == "$not":
+            yield key, dict(_add_prefix(value))
         elif "." in key and key.split(".", 1)[0] in ("sp", "doc"):
             yield key, value
         elif key in ("sp", "doc"):

tests/conftest.py

@@ -6,3 +6,34 @@
 @pytest.fixture
 def testdata():
     return str(uuid.uuid4())
+
+
+@pytest.fixture
+def find_filter():
+    return [
+        {"a": 0},
+        {"a.b": 0},
+        {"a.b": {"$lt": 42}},
+        {"a.b.$lt": 42},
+        {"$or": [{"a.b": 41}, {"a.b.$lt": 42}]},
+        {"$or": [{"a.b": 42}, {"a.b.$lt": 42}]},
+        {"$and": [{"a.b": 42}, {"a.b.$lt": 42}]},
+        {"$and": [{"a.b": 0}, {"a.b.$lt": 42}]},
+        {"$and": [{"a.b.$gte": 0}, {"a.b.$lt": 42}]},
+        {"$not": {"a.b": 0}},
+        {"$and": [{"a.b.$gte": 0}, {"$not": {"a.b.$lt": 42}}]},
+        {"$not": {"$not": {"a.b": 0}}},
+        {"a.b": {"$in": [0, 1]}},
+        {"a.b": {"$nin": [0, 1]}},
+        {"$not": {"a.b": {"$in": [0, 1]}}},
+        {"a.b": {"$exists": True}},
+        {"a.b": {"$exists": False}},
+        {"a": {"$exists": True}},
+        {"a": {"$exists": False}},
+        {"c": {"$regex": r"^\d$"}},
+        {"c": {"$type": "str"}},
+        {"d": {"$type": "list"}},
+        {"a.b": {"$where": "lambda x: x < 10"}},
+        {"a.b": {"$where": "lambda x: isinstance(x, int)"}},
+        {"a": {"$regex": "[a][b][c]"}},
+    ]

tests/test_find_command_line_interface.py

@@ -11,35 +11,6 @@
 
 from signac.filterparse import parse_filter_arg, parse_simple
 
-FILTERS = [
-    {"a": 0},
-    {"a.b": 0},
-    {"a.b": {"$lt": 42}},
-    {"a.b.$lt": 42},
-    {"$or": [{"a.b": 41}, {"a.b.$lt": 42}]},
-    {"$or": [{"a.b": 42}, {"a.b.$lt": 42}]},
-    {"$and": [{"a.b": 42}, {"a.b.$lt": 42}]},
-    {"$and": [{"a.b": 0}, {"a.b.$lt": 42}]},
-    {"$and": [{"a.b.$gte": 0}, {"a.b.$lt": 42}]},
-    {"$not": {"a.b": 0}},
-    {"$and": [{"a.b.$gte": 0}, {"$not": {"a.b.$lt": 42}}]},
-    {"$not": {"$not": {"a.b": 0}}},
-    {"a.b": {"$in": [0, 1]}},
-    {"a.b": {"$nin": [0, 1]}},
-    {"$not": {"a.b": {"$in": [0, 1]}}},
-    {"a.b": {"$exists": True}},
-    {"a.b": {"$exists": False}},
-    {"a": {"$exists": True}},
-    {"a": {"$exists": False}},
-    {"c": {"$regex": r"^\d$"}},
-    {"c": {"$type": "str"}},
-    {"d": {"$type": "list"}},
-    {"a.b": {"$where": "lambda x: x < 10"}},
-    {"a.b": {"$where": "lambda x: isinstance(x, int)"}},
-    {"a": {"$regex": "[a][b][c]"}},
-]
-
-
 VALUES = {"1": 1, "1.0": 1.0, "abc": "abc", "true": True, "false": False, "null": None}
 
 ARITHMETIC_EXPRESSIONS = [
@@ -68,20 +39,24 @@ def _parse(args):
         with redirect_stderr(StringIO()):
             return parse_filter_arg(args)
 
-    def test_interpret_json(self):
+    @pytest.mark.usefixtures("find_filter")
+    def test_interpret_json(self, find_filter):
         def _assert_equal(q):
+            # TODO: full code path not tested with this test.
+            # _assert_equal and _find_expression, are not tested
             assert q == self._parse([json.dumps(q)])
 
-        for f in FILTERS:
+        for f in find_filter:
             _assert_equal(f)
 
-    def test_interpret_simple(self):
+    @pytest.mark.usefixtures("find_filter")
+    def test_interpret_simple(self, find_filter):
         assert self._parse(["a"]) == {"a": {"$exists": True}}
         assert next(parse_simple(["a"])) == ("a", {"$exists": True})
 
         for s, v in VALUES.items():
             assert self._parse(["a", s]) == {"a": v}
-        for f in FILTERS:
+        for f in find_filter:
             f_ = f.copy()
             key, value = f.popitem()
             if key.startswith("$"):

tests/test_shell.py

@@ -217,7 +217,8 @@ def test_view_incomplete_path_spec(self):
         )
         assert "duplicate paths" in err
 
-    def test_find(self):
+    @pytest.mark.usefixtures("find_filter")
+    def test_find(self, find_filter):
         self.call("python -m signac init".split())
         project = signac.Project()
         sps = [{"a": i} for i in range(3)]
@@ -280,6 +281,12 @@ def test_find(self):
                 == [job.id for job in project.find_jobs({"doc.b": i})][0]
             )
 
+        # ensure that there are no errors due to adding sp and doc prefixes
+        # by testing on all the example complex expressions
+        for f in find_filter:
+            command = "python -m signac find ".split() + [json.dumps(f)]
+            self.call(command).strip()
+
     def test_diff(self):
         self.call("python -m signac init".split())
         project = signac.Project()