Streamline transformer and test not-implemented

aiida_optimade/main.py

@@ -64,6 +64,7 @@
 )
 APP.add_exception_handler(ValidationError, exc_handlers.validation_exception_handler)
 APP.add_exception_handler(VisitError, exc_handlers.grammar_not_implemented_handler)
+APP.add_exception_handler(NotImplementedError, exc_handlers.not_implemented_handler)
 APP.add_exception_handler(Exception, exc_handlers.general_exception_handler)
 
 

aiida_optimade/transformers/aiida.py

@@ -48,7 +48,7 @@ def value(self, value):
     @v_args(inline=True)
     def non_string_value(self, value):
         """ non_string_value: number | property """
-        # Note: Do nothing!
+        # NOTE: Do nothing!
         return value
 
     @v_args(inline=True)
@@ -63,26 +63,16 @@ def not_implemented_string(self, value):
 
     def value_list(self, args):
         """value_list: [ OPERATOR ] value ( "," [ OPERATOR ] value )*"""
-        values = []
         for value in args:
-            if value in self.reversed_operator_map:
+            if str(value) in self.reversed_operator_map:
                 # value is OPERATOR
                 # This is currently not supported
                 raise NotImplementedError(
                     f"OPERATOR {value} inside value_list {args} has not been "
                     "implemented."
                 )
 
-            try:
-                value = float(value)
-            except ValueError:
-                if value.startswith('"') and value.endswith('"'):
-                    value = value[1:-1]
-            else:
-                if value.is_integer():
-                    value = int(value)
-            values.append(value)
-        return values
+        return args
 
     def value_zip(self, arg):
         """
@@ -170,18 +160,15 @@ def fuzzy_string_op_rhs(self, arg):
                              STARTS [ WITH ] string |
                              ENDS [ WITH ] string
         """
-        # The WITH keyword may be omitted.
-        if isinstance(arg[1], Token) and arg[1].type == "WITH":
-            pattern = arg[2]
-        else:
-            pattern = arg[1]
-
+        # Since the string pattern will always be the last argument,
+        # and there is always another keyword before the OPTIONAL "WITH",
+        # there is no need to test for the existence of "WITH"
         if arg[0] == "CONTAINS":
-            like = f"%{pattern}%"
+            like = f"%{arg[-1]}%"
         elif arg[0] == "STARTS":
-            like = f"{pattern}%"
+            like = f"{arg[-1]}%"
         elif arg[0] == "ENDS":
-            like = f"%{pattern}"
+            like = f"%{arg[-1]}"
         return {"like": like}
 
     def set_op_rhs(self, arg):
@@ -200,13 +187,11 @@ def set_op_rhs(self, arg):
         if arg[1] == "ANY":
             return {"or": [{"contains": [value]} for value in arg[2]]}
         if arg[1] == "ONLY":
-            raise NotImplementedError(
-                "'set_op_rhs: HAS ONLY value_list' has not been implemented."
-            )
+            raise NotImplementedError
 
         # value with OPERATOR
         raise NotImplementedError(
-            "'set_op_rhs: HAS OPERATOR value' has not been implemented."
+            f"set_op_rhs has not been implemented for use with OPERATOR. Given: {arg}"
         )
 
     def set_zip_op_rhs(self, arg):
@@ -239,8 +224,7 @@ def length_op_rhs(self, arg):
             }
 
         raise NotImplementedError(
-            f"length_comparison has failed with {arg}. "
-            "Unknown not-implemented operator."
+            f"Operator {operator} has not been implemented for the LENGTH filter."
         )
 
     def property_zip_addon(self, arg):
@@ -280,7 +264,7 @@ def number(self, number):
             type_ = float
         return type_(number)
 
-    def __default__(self, data, children, meta):
+    def __default__(self, data, children, meta):  # pragma: no cover
         raise NotImplementedError(
             "Calling __default__, i.e., unknown grammar concept. "
             f"data: {data}, children: {children}, meta: {meta}"

tests/server/conftest.py

@@ -55,6 +55,7 @@ def inner(
         expected_title: str = None,
         expected_detail: str = None,
     ):
+        response = None
         try:
             response = client.get(request)
             assert response.status_code == expected_status, (
@@ -64,22 +65,28 @@ def inner(
             )
 
             response = response.json()
-            assert len(response["errors"]) == 1
-            assert response["meta"]["data_returned"] == 0
+            assert len(response["errors"]) == 1, response.get(
+                "errors", "'errors' not found"
+            )
+            assert response["meta"]["data_returned"] == 0, response.get(
+                "meta", "'meta' not found"
+            )
 
             error = response["errors"][0]
-            assert str(expected_status) == error["status"]
-            assert expected_title == error["title"]
+            assert str(expected_status) == error["status"], error
+            assert expected_title == error["title"], error
 
             if expected_detail is None:
                 expected_detail = "Error trying to process rule "
-                assert error["detail"].startswith(expected_detail)
+                assert error["detail"].startswith(expected_detail), error
             else:
-                assert expected_detail == error["detail"]
+                assert expected_detail == error["detail"], error
 
         except Exception as exc:
             print("Request attempted:")
             print(f"{client.base_url}{request}")
+            if response:
+                print(f"\nCaptured response:\n{response}")
             raise exc
 
     return inner

tests/server/test_query_params.py

@@ -115,7 +115,7 @@ def test_list_length_basic(check_response):
     check_response(request, expected_ids)
 
 
-def test_list_length(check_response):
+def test_list_length_operators(check_response):
     request = "/structures?filter=elements LENGTH = 17"
     expected_ids = ["1047"]
     check_response(request, expected_ids)
@@ -129,18 +129,35 @@ def test_list_length(check_response):
     check_response(request, expected_ids)
 
 
-@pytest.mark.skip("HAS ONLY is not implemented.")
-def test_list_has_only(check_response):
+def test_list_length_bad_operators(check_error_response):
+    """Check NonImplementedError is raised when using a valid,
+    but not-supported operator"""
+    bad_valid_operator = "!="
+    request = f"/structures?filter=elements LENGTH {bad_valid_operator} 2"
+    check_error_response(
+        request,
+        expected_status=501,
+        expected_title="NotImplementedError",
+        expected_detail=(
+            f"Operator {bad_valid_operator} has not been implemented for the LENGTH filter."
+        ),
+    )
+
+
+def test_list_has_only(check_error_response):
+    # HAS ONLY is not yet implemented
     request = '/structures?filter=elements HAS ONLY "Ac"'
-    expected_ids = [""]
-    check_response(request, expected_ids)
+    check_error_response(
+        request, expected_status=501, expected_title="NotImplementedError"
+    )
 
 
-@pytest.mark.skip("Zips are not implemented.")
-def test_list_correlated(check_response):
+def test_list_correlated(check_error_response):
+    # Zipped lists are not yet implemented
     request = '/structures?filter=elements:elements_ratios HAS "Ag":"0.2"'
-    expected_ids = [""]
-    check_response(request, expected_ids)
+    check_error_response(
+        request, expected_status=501, expected_title="NotImplementedError"
+    )
 
 
 def test_saved_extras_is_known(check_response):