Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parsing of $not expression on command line #970

Merged
merged 9 commits into from
Feb 2, 2024
5 changes: 5 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------

Expand Down
3 changes: 3 additions & 0 deletions signac/filterparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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":
cbkerr marked this conversation as resolved.
Show resolved Hide resolved
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"):
Expand Down
31 changes: 31 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]"}},
]
41 changes: 8 additions & 33 deletions tests/test_find_command_line_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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("$"):
Expand Down
9 changes: 8 additions & 1 deletion tests/test_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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()
Expand Down
Loading