Repository: glotzerlab/signac

Issue:
Using signac find on the CLI for tuples
<!-- Please replace the text in the individual sections below. -->

### Description

Finding jobs for which a given statepoint parameter, x, is equal to a particular list cannot be as easily done with the `signac find` CLI option as it can with other statepoint parameters. For example, `signac find x '[0, 1, 0]'` is currently interpreted as `'{"x": "[0, 1, 0]"}'` . The user has to explicitly specify `signac find '{"x": [0, 1, 0]}'` to get around this.

### To reproduce

`signac find x '[0, 1, 0]'` will return no jobs, even if x = [0, 1, 0] for some of them

### Error output

No error is thrown. The command is only misinterpreted.

### System configuration

Please complete the following information:

 - Operating System [e.g. macOS]: macOS
 - Version of Python [e.g. 3.7]: 3.9.6
 - Version of signac [e.g. 1.0]: 1.7.0

Or copy & paste the output of: `python -c 'import platform; print(platform.platform()); import sys; print(sys.version); import signac; print(signac.__version__)'`


Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) Report
> Merging [#622](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (e4a4411) into [master](https://codecov.io/gh/glotzerlab/signac/commit/42986037f07766ac1278716d4256da17690747e8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) (4298603) will **decrease** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/glotzerlab/signac/pull/622/graphs/tree.svg?width=650&height=150&src=pr&token=FghbjTyzgM&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)

```diff
@@            Coverage Diff             @@
##           master     #622      +/-   ##
==========================================
- Coverage   78.31%   78.30%   -0.01%     
==========================================
  Files          65       65              
  Lines        7079     7081       +2     
  Branches     1310     1310              
==========================================
+ Hits         5544     5545       +1     
  Misses       1227     1227              
- Partials      308      309       +1     
```


| [Impacted Files](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab) | Coverage Δ | |
|---|---|---|
| [signac/contrib/filterparse.py](https://codecov.io/gh/glotzerlab/signac/pull/622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL2NvbnRyaWIvZmlsdGVycGFyc2UucHk=) | `84.84% <100.00%> (+0.31%)` | :arrow_up: |
| [signac/\_\_main\_\_.py](https://codecov.io/gh/glotzerlab/signac/pull/622/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab#diff-c2lnbmFjL19fbWFpbl9fLnB5) | `71.78% <0.00%> (-0.12%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab). Last update [4298603...e4a4411](https://codecov.io/gh/glotzerlab/signac/pull/622?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=glotzerlab).

AlainKadar: Can I remove the `test_invalid_json` test in test_find_command_line_interface.py? The CLI works fine with the changes; passing an invalid JSON still prints "Failed to parse query argument. Ensure that '{"x": True}' is valid JSON!" but pytest reports the error is not raised. This is because I no longer allow invalid JSONs to be passed to `_parse_json()` ? 
bdice: @AlainKadar Changing the tests should be avoided. In the changes @vyasr suggested, we lose one feature that I hadn't anticipated: we have no way to tell the user when they have provided invalid JSON, because we just treat it as a string if it cannot be parsed as JSON. The previous code attempted to recognize JSON-like data, which would catch invalid JSON like `{"x": True}` -- note that JSON uses `true` instead of `True`.

@vyasr Would you be open to reverting the eager parsing and returning to the previous approach where we attempt to detect JSON-like inputs? I can certainly imagine that our invalid JSON detection has saved a lot of users who may not be familiar with how the format differs from Python literals -- it has caught my mistakes several times.
vyasr: > @vyasr Would you be open to reverting the eager parsing and returning to the previous approach where we attempt to detect JSON-like inputs? I can certainly imagine that our invalid JSON detection has saved a lot of users who may not be familiar with how the format differs from Python literals -- it has caught my mistakes several times.

That's fair, I hadn't thought that would be too important but I'm happy to preserve if if it's helpful to people. In that case, we should rename the function something like `_is_json_like` or `_may_be_json` to indicate that returning true here is a necessary but not sufficient condition for the input to be JSON.
AlainKadar: Style changes made, thanks for the advice

Patch:
diff --git a/signac/contrib/filterparse.py b/signac/contrib/filterparse.py
index 0fe52b75..9868fb8d 100644
--- a/signac/contrib/filterparse.py
+++ b/signac/contrib/filterparse.py
@@ -62,8 +62,8 @@ def _read_index(project, fn_index=None):
         return (json.loads(line) for line in fd)
 
 
-def _is_json(q):
-    """Check if q is JSON.
+def _is_json_like(q):
+    """Check if q is JSON like.
 
     Parameters
     ----------
@@ -76,7 +76,7 @@ def _is_json(q):
         True if q starts with "{" and ends with "}".
 
     """
-    return q.strip().startswith("{") and q.strip().endswith("}")
+    return (q[0] == "{" and q[-1] == "}") or (q[0] == "[" and q[-1] == "]")
 
 
 def _is_regex(q):
@@ -179,19 +179,18 @@ def _parse_single(key, value=None):
     ------
     ValueError
         If filter arguments have an invalid key.
-
     """
-    if value is None or value == "!":
+    if _is_json_like(key):
+        raise ValueError(
+            "Please check your filter arguments. "
+            f"Using a JSON expression as a key is not allowed: '{key}'."
+        )
+    elif value is None or value == "!":
         return key, {"$exists": True}
-    elif _is_json(value):
+    elif _is_json_like(value):
         return key, _parse_json(value)
     elif _is_regex(value):
         return key, {"$regex": value[1:-1]}
-    elif _is_json(key):
-        raise ValueError(
-            "Please check your filter arguments. "
-            "Using a JSON expression as a key is not allowed: '{}'.".format(key)
-        )
     else:
         return key, _cast(value)
 
@@ -238,14 +237,13 @@ def parse_filter_arg(args, file=sys.stderr):
     if args is None or len(args) == 0:
         return None
     elif len(args) == 1:
-        if _is_json(args[0]):
+        if _is_json_like(args[0]):
             return _parse_json(args[0])
         else:
             key, value = _parse_single(args[0])
             return _with_message({key: value}, file)
     else:
         q = dict(parse_simple(args))
-
         return _with_message(q, file)
 
 


Test patch:
diff --git a/tests/test_shell.py b/tests/test_shell.py
index 6ea63bd4..1c5f62de 100644
--- a/tests/test_shell.py
+++ b/tests/test_shell.py
@@ -239,6 +239,7 @@ class TestBasicShell:
         self.call("python -m signac init my_project".split())
         project = signac.Project()
         sps = [{"a": i} for i in range(3)]
+        sps.append({"a": [0, 1, 0]})
         for sp in sps:
             project.open_job(sp).init()
         out = self.call("python -m signac find".split())
@@ -269,8 +270,15 @@ class TestBasicShell:
         assert '{"a": 0}' in out
         assert '{"a": 2}' in out
 
+        job = project.open_job({"a": [0, 1, 0]})
+        msg = [*"python -m signac find a".split(), "[0, 1, 0]", "--sp"]
+        out = self.call(msg).strip()
+        assert out.strip().split(os.linesep) == [str(job.id), str(job.statepoint)]
+
         # Test the doc_filter
         for job in project.find_jobs():
+            if job.statepoint()["a"] == [0, 1, 0]:
+                continue
             job.document["a"] = job.statepoint()["a"]
             job.document["b"] = job.statepoint()["a"] + 1