major refactor + sort + pagination + tests to other explorers

Author: esoteric-ephemera

.github/workflows/testing.yml

@@ -62,7 +62,7 @@ jobs:
           MP_API_KEY: ${{ secrets[env.API_KEY_NAME] }}
           # MP_API_ENDPOINT: https://api-preview.materialsproject.org/
         run: |
-          pytest -n auto -x --cov=mp_api --cov-report=xml
+          pytest -n auto -x --cov=mp_api --omit=mp_api/_test_utils.py --cov-report=xml
       - uses: codecov/codecov-action@v1
         with:
           token: ${{ secrets.CODECOV_TOKEN }}

mp_api/_test_utils.py

@@ -0,0 +1,114 @@
+"""Define testing utils that need to imported."""
+
+from __future__ import annotations
+
+try:
+    import pytest
+except ImportError as exc:
+    raise ImportError(
+        "You must `pip install 'mp-api[test]' to use these testing utilities."
+    ) from exc
+
+import os
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from collections.abc import Callable, Sequence
+    from typing import Any
+
+requires_api_key = pytest.mark.skipif(
+    os.getenv("MP_API_KEY") is None,
+    reason="No API key found.",
+)
+
+NUM_DOCS = 5
+
+def client_search_testing(
+    search_method: Callable,
+    excluded_params: list[str],
+    alt_name_dict: dict[str, str],
+    custom_field_tests: dict[str, Any],
+    sub_doc_fields: list[str],
+    int_bounds : tuple[int,int] = (-100, 100),
+    float_bounds : tuple[float,float] = (-100.12, 100.12),
+):
+    """
+    Function to test a client using its search method.
+    Each parameter is used to query for data, which is then checked.
+
+    Args:
+        search_method (Callable): Client search method
+        excluded_params (list[str]): List of parameters to exclude from testing
+        alt_name_dict (dict[str, str]): Alternative names for parameters used in the projection and subsequent data checking
+        custom_field_tests (dict[str, Any]): Custom queries for specific fields.
+        sub_doc_fields (list[str]): Prefixes for fields to check in resulting data. Useful when data to be tested is nested.
+        int_bounds (tuple[int,int]) : integer bounds to use in testing int-type query arguments
+        float_boudns (tuple[float,float]) : float bounds to use in testing float-type query arguments
+    """
+    if search_method is None:
+        return
+    # Get list of parameters
+    param_tuples = list(search_method.__annotations__.items())
+
+    # Query API for each numeric and boolean parameter and check if returned
+    for entry in param_tuples:
+        param = entry[0]
+
+        if param not in excluded_params + ["return"]:
+            param_type = entry[1]
+            q = {"chunk_size": 1, "num_chunks": 1}
+
+            if "tuple[int, int]" in param_type:
+                q[param] = int_bounds
+            elif "tuple[float, float]" in param_type:
+                q[param] = float_bounds
+            elif "bool" in param_type:
+                q[param] = False
+            elif param in custom_field_tests:
+                q[param] = custom_field_tests[param]
+            else:
+                raise ValueError(
+                    f"Parameter '{param}' with type '{param_type}' was not "
+                    "properly identified in the generic search method test."
+                )
+
+            if len(docs := search_method(**q)) > 0:
+                doc = docs[0].model_dump()
+            else:
+                raise ValueError("No documents returned")
+
+            for sub_field in sub_doc_fields:
+                if sub_field in doc:
+                    doc = doc[sub_field]
+
+            assert doc[alt_name_dict.get(param, param)] is not None
+
+
+def client_pagination(search_method : Callable, id_name : str):
+
+    page_1 = search_method(_page=1, chunk_size=NUM_DOCS, fields = [id_name])
+    page_2 = search_method(_page=2, chunk_size=NUM_DOCS, fields = [id_name])
+    assert all(
+        len(results) == NUM_DOCS for results in (page_1, page_2)
+    )
+    assert {str(getattr(doc, id_name)) for doc in page_1}.intersection(
+        {str(getattr(doc,id_name)) for doc in page_2}
+    ) == set()
+
+
+def client_sort(search_method : Callable, sort_fields : str | Sequence[str]):
+
+    for sort_field in ([sort_fields] if isinstance(sort_fields,str) else sort_fields):
+        asc = search_method(_page=1, _sort_fields=sort_field, chunk_size=NUM_DOCS, fields = [sort_field])
+        desc = search_method(_page=1, _sort_fields=f"-{sort_field}", chunk_size=NUM_DOCS, fields = [sort_field])
+
+        idxs = list(range(NUM_DOCS))
+        assert sorted(
+            idxs, key=lambda idx: getattr(asc[idx],sort_field)
+        ) == idxs
+
+        assert sorted(
+            idxs,
+            key=lambda idx: getattr(desc[idx],sort_field),
+            reverse=True,
+        ) == idxs

mp_api/client/routes/materials/electrodes.py

@@ -87,57 +87,33 @@ def search(
         """
         query_params: dict = defaultdict(dict)
 
-        if battery_ids:
-            if isinstance(battery_ids, str):
-                battery_ids = [battery_ids]
-
-            query_params.update({"battery_ids": ",".join(validate_ids(battery_ids))})
-
-        if working_ion:
-            if isinstance(working_ion, (str, Element)):
-                working_ion = [working_ion]  # type: ignore
-
-            query_params.update(
-                {"working_ion": ",".join([str(ele) for ele in working_ion])}  # type: ignore
-            )
-
-        if formula:
-            if isinstance(formula, str):
-                formula = [formula]
-
-            query_params.update({"formula": ",".join(formula)})
-
-        if elements:
-            query_params.update({"elements": ",".join(elements)})
-
-        if num_elements:
-            if isinstance(num_elements, int):
-                num_elements = (num_elements, num_elements)
-            query_params.update(
-                {"nelements_min": num_elements[0], "nelements_max": num_elements[1]}
-            )
-
-        if exclude_elements:
-            query_params.update({"exclude_elements": ",".join(exclude_elements)})
-
         for param, value in locals().items():
             if (
-                param
-                not in [
-                    "__class__",
-                    "self",
-                    "working_ion",
-                    "query_params",
-                    "num_elements",
-                ]
-                and value
-            ):
-                if isinstance(value, tuple):
+                param not in {"__class__", "self", "query_params"}
+                and value is not None
+            ):       
+
+                if param == "num_elements": # this must come first
+                    if isinstance(num_elements, int):
+                        num_elements = (num_elements, num_elements)
+                    query_params.update(
+                        {"nelements_min": num_elements[0], "nelements_max": num_elements[1]}
+                    ) 
+                                
+                elif isinstance(value, tuple):
                     query_params.update(
                         {f"{param}_min": value[0], f"{param}_max": value[1]}
                     )
+                elif param == "battery_ids":
+                    query_params[param] = ",".join(validate_ids(value))
+                elif param == "working_ion":
+                    query_params["working_ion"] = ",".join(
+                        str(ele) for ele in ([value] if isinstance(value, str | Element) else value)
+                    )
+                elif param in ("formula","elements","exclude_elements"):
+                    query_params[param] = ",".join([value] if isinstance(value,str) else value)
                 else:
-                    query_params.update({param: value})
+                    query_params[param] = value
 
         excluded_fields = self._exclude_search_fields or []
         ignored_fields = {
@@ -181,4 +157,6 @@ class ConversionElectrodeRester(BaseElectrodeRester):
         "stability_charge",
         "stability_discharge",
         "exclude_elements",
+        "_page",
+        "_sort_fields",
     ]

mp_api/client/routes/materials/summary.py

@@ -206,8 +206,9 @@ def search(  # noqa: D417
         mmnd_inv = {v: k for k, v in min_max_name_dict.items() if k != v}
 
         # Set user query params from `locals`
+        _locals = locals()
         user_settings = {
-            k: v for k, v in locals().items() if k in min_max_name_dict and v
+            k: v for k, v in _locals.items() if k in min_max_name_dict and v is not None
         }
 
         # Check to see if user specified _search fields using **kwargs,
@@ -290,7 +291,7 @@ def _csrc(x):
                 )
 
         for param, value in user_settings.items():
-            if param.startswith("_"):
+            if param in {"_page", "_sort_fields"}:
                 query_params[param] = value
             else:
                 if isinstance(value, (int, float)):
@@ -308,30 +309,10 @@ def _csrc(x):
 
             query_params.update({"material_ids": ",".join(validate_ids(material_ids))})
 
-        if deprecated is not None:
-            query_params.update({"deprecated": deprecated})
-
-        if formula:
-            if isinstance(formula, str):
-                formula = [formula]
-
-            query_params.update({"formula": ",".join(formula)})
-
-        if chemsys:
-            if isinstance(chemsys, str):
-                chemsys = [chemsys]
-
-            query_params.update({"chemsys": ",".join(chemsys)})
-
-        if elements:
-            query_params.update({"elements": ",".join(elements)})
-
-        if exclude_elements is not None:
-            query_params.update({"exclude_elements": ",".join(exclude_elements)})
-
-        if possible_species is not None:
-            query_params.update({"possible_species": ",".join(possible_species)})
-
+        for k in ("formula", "chemsys", "elements", "exclude_elements", "possible_species"):
+            if (v := _locals.get(k)) is not None:
+                query_params[k] = ",".join([v] if isinstance(v,str) else v)
+                
         symm_cardinality = {
             "crystal_system": 7,
             "spacegroup_number": 230,
@@ -350,21 +331,13 @@ def _csrc(x):
             else:
                 query_params.update({k: symm_vals})
 
-        if is_stable is not None:
-            query_params.update({"is_stable": is_stable})
-
-        if is_gap_direct is not None:
-            query_params.update({"is_gap_direct": is_gap_direct})
-
-        if is_metal is not None:
-            query_params.update({"is_metal": is_metal})
+        for k in ("deprecated", "is_stable", "is_gap_direct", "is_metal", "has_reconstructed", "theoretical"):
+            if (v := _locals.get(k)) is not None:
+                query_params[k] = v
 
         if magnetic_ordering:
             query_params.update({"ordering": magnetic_ordering.value})
 
-        if has_reconstructed is not None:
-            query_params.update({"has_reconstructed": has_reconstructed})
-
         if has_props:
             has_props_clean = []
             for prop in has_props:
@@ -375,9 +348,6 @@ def _csrc(x):
 
             query_params.update({"has_props": ",".join(has_props_clean)})
 
-        if theoretical is not None:
-            query_params.update({"theoretical": theoretical})
-
         if not include_gnome:
             query_params.update({"batch_id_not_eq": "gnome_r2scan_statics"})
 

mp_api/client/routes/materials/xas.py

@@ -33,6 +33,8 @@ def search(
         chunk_size: int = 1000,
         all_fields: bool = True,
         fields: list[str] | None = None,
+        _page: int | None = None,
+        _sort_fields: str | None = None,
     ):
         """Query core XAS docs using a variety of search criteria.
 
@@ -54,14 +56,19 @@ def search(
             all_fields (bool): Whether to return all fields in the document. Defaults to True.
             fields (List[str]): List of fields in MaterialsCoreDoc to return data for.
                 Default is material_id, last_updated, and formula_pretty if all_fields is False.
+            _page (int or None) : Page of the results to skip to.
+            _sort_fields (str or None) : Field to sort on. Including a leading "-" sign will reverse sort order.
 
         Returns:
             ([MaterialsDoc]) List of material documents
         """
-        query_params: dict[str, Any] = {}
 
-        if edge:
-            query_params.update({"edge": edge})
+        _locals = locals()
+        query_params: dict[str, Any] = {
+            k : _locals[k]
+            for k in ("edge", "spectrum_type", "formula", "_page", "_sort_fields")
+            if _locals.get(k) is not None
+        }
 
         if absorbing_element:
             query_params.update(
@@ -71,33 +78,10 @@ def search(
                     else absorbing_element
                 }
             )
-
-        if spectrum_type:
-            query_params.update({"spectrum_type": spectrum_type})
-
-        if formula:
-            query_params.update({"formula": formula})
-
-        if chemsys:
-            if isinstance(chemsys, str):
-                chemsys = [chemsys]
-
-            query_params.update({"chemsys": ",".join(chemsys)})
-
-        if elements:
-            query_params.update({"elements": ",".join(elements)})
-
-        if material_ids:
-            if isinstance(material_ids, str):
-                material_ids = [material_ids]
-
-            query_params.update({"material_ids": ",".join(validate_ids(material_ids))})
-
-        if spectrum_ids:
-            if isinstance(spectrum_ids, str):
-                spectrum_ids = [spectrum_ids]
-
-            query_params.update({"spectrum_ids": ",".join(spectrum_ids)})
+        for k in ("chemsys", "elements", "material_ids", "spectrum_ids"):
+            if (v := _locals.get(k) ) is not None:
+                query_params[k] = ",".join([v] if isinstance(v,str) else v)
+                
 
         query_params = {
             entry: query_params[entry]