settings var + jcesr typo fix

Author: esoteric-ephemera

mp_api/client/core/__init__.py

@@ -2,4 +2,4 @@
 
 from .client import BaseRester
 from .exceptions import MPRestError, MPRestWarning
-from .settings import MAPIClientSettings
+from .settings import MAPI_CLIENT_SETTINGS, MAPIClientSettings

mp_api/client/core/client.py

@@ -19,7 +19,7 @@
 from json import JSONDecodeError
 from math import ceil
 from typing import TYPE_CHECKING, ForwardRef, Optional, get_args
-from urllib.parse import quote, urljoin
+from urllib.parse import quote
 
 import requests
 from emmet.core.utils import jsanitize
@@ -31,8 +31,13 @@
 from urllib3.util.retry import Retry
 
 from mp_api.client.core.exceptions import MPRestError
-from mp_api.client.core.settings import MAPIClientSettings
-from mp_api.client.core.utils import load_json, validate_ids
+from mp_api.client.core.settings import MAPI_CLIENT_SETTINGS
+from mp_api.client.core.utils import (
+    load_json,
+    validate_api_key,
+    validate_endpoint,
+    validate_ids,
+)
 
 try:
     import boto3
@@ -59,9 +64,6 @@
     __version__ = os.getenv("SETUPTOOLS_SCM_PRETEND_VERSION")
 
 
-SETTINGS = MAPIClientSettings()  # type: ignore
-
-
 class _DictLikeAccess(BaseModel):
     """Define a pydantic mix-in which permits dict-like access to model fields."""
 
@@ -98,7 +100,7 @@ def __init__(
         use_document_model: bool = True,
         timeout: int = 20,
         headers: dict | None = None,
-        mute_progress_bars: bool = SETTINGS.MUTE_PROGRESS_BARS,
+        mute_progress_bars: bool = MAPI_CLIENT_SETTINGS.MUTE_PROGRESS_BARS,
         **kwargs,
     ):
         """Initialize the REST API helper class.
@@ -132,23 +134,17 @@ def __init__(
             mute_progress_bars: Whether to disable progress bars.
             **kwargs: access to legacy kwargs that may be in the process of being deprecated
         """
-        # TODO: think about how to migrate from PMG_MAPI_KEY
-        self.api_key = api_key or os.getenv("MP_API_KEY")
-        self.base_endpoint = self.endpoint = endpoint or os.getenv(
-            "MP_API_ENDPOINT", "https://api.materialsproject.org/"
-        )
+        self.api_key = validate_api_key(api_key)
+        self.base_endpoint = validate_endpoint(endpoint)
+        self.endpoint = validate_endpoint(endpoint, suffix=self.suffix)
+
         self.debug = debug
         self.include_user_agent = include_user_agent
         self.use_document_model = use_document_model
         self.timeout = timeout
         self.headers = headers or {}
         self.mute_progress_bars = mute_progress_bars
-        self.db_version = BaseRester._get_database_version(self.endpoint)
-
-        if self.suffix:
-            self.endpoint = urljoin(self.endpoint, self.suffix)
-        if not self.endpoint.endswith("/"):
-            self.endpoint += "/"
+        self.db_version = BaseRester._get_database_version(self.base_endpoint)
 
         self._session = session
         self._s3_client = s3_client
@@ -196,15 +192,14 @@ def _create_session(api_key, include_user_agent, headers):
             user_agent = f"{mp_api_info} ({python_info} {platform_info})"
             session.headers["user-agent"] = user_agent
 
-        settings = MAPIClientSettings()  # type: ignore
-        max_retry_num = settings.MAX_RETRIES
+        max_retry_num = MAPI_CLIENT_SETTINGS.MAX_RETRIES
         retry = Retry(
             total=max_retry_num,
             read=max_retry_num,
             connect=max_retry_num,
             respect_retry_after_header=True,
             status_forcelist=[429, 504, 502],  # rate limiting
-            backoff_factor=settings.BACKOFF_FACTOR,
+            backoff_factor=MAPI_CLIENT_SETTINGS.BACKOFF_FACTOR,
         )
         adapter = HTTPAdapter(max_retries=retry)
         session.mount("http://", adapter)
@@ -265,11 +260,7 @@ def _post_resource(
         payload = jsanitize(body)
 
         try:
-            url = self.endpoint
-            if suburl:
-                url = urljoin(self.endpoint, suburl)
-                if not url.endswith("/"):
-                    url += "/"
+            url = validate_endpoint(self.endpoint, suffix=suburl)
             response = self.session.post(url, json=payload, verify=True, params=params)
 
             if response.status_code == 200:
@@ -333,11 +324,7 @@ def _patch_resource(
         payload = jsanitize(body)
 
         try:
-            url = self.endpoint
-            if suburl:
-                url = urljoin(self.endpoint, suburl)
-                if not url.endswith("/"):
-                    url += "/"
+            url = validate_endpoint(self.endpoint, suffix=suburl)
             response = self.session.patch(url, json=payload, verify=True, params=params)
 
             if response.status_code == 200:
@@ -469,11 +456,7 @@ def _query_resource(
             criteria["_fields"] = ",".join(fields)
 
         try:
-            url = self.endpoint
-            if suburl:
-                url = urljoin(self.endpoint, suburl)
-                if not url.endswith("/"):
-                    url += "/"
+            url = validate_endpoint(self.endpoint, suffix=suburl)
 
             if query_s3:
                 db_version = self.db_version.replace(".", "-")
@@ -620,15 +603,15 @@ def _submit_requests(  # noqa
 
             bare_url_len = len(url_string)
             max_param_str_length = (
-                MAPIClientSettings().MAX_HTTP_URL_LENGTH - bare_url_len  # type: ignore
+                MAPI_CLIENT_SETTINGS.MAX_HTTP_URL_LENGTH - bare_url_len  # type: ignore
             )
 
             # Next, check if default number of parallel requests works.
             # If not, make slice size the minimum number of param entries
             # contained in any substring of length max_param_str_length.
             param_length = len(criteria[parallel_param].split(","))
             slice_size = (
-                int(param_length / MAPIClientSettings().NUM_PARALLEL_REQUESTS) or 1  # type: ignore
+                int(param_length / MAPI_CLIENT_SETTINGS.NUM_PARALLEL_REQUESTS) or 1  # type: ignore
             )
 
             url_param_string = quote(criteria[parallel_param])
@@ -909,14 +892,14 @@ def _multi_thread(
         params_ind = 0
 
         with ThreadPoolExecutor(
-            max_workers=MAPIClientSettings().NUM_PARALLEL_REQUESTS  # type: ignore
+            max_workers=MAPI_CLIENT_SETTINGS.NUM_PARALLEL_REQUESTS  # type: ignore
         ) as executor:
             # Get list of initial futures defined by max number of parallel requests
             futures = set()
 
             for params in itertools.islice(
                 params_gen,
-                MAPIClientSettings().NUM_PARALLEL_REQUESTS,  # type: ignore
+                MAPI_CLIENT_SETTINGS.NUM_PARALLEL_REQUESTS,  # type: ignore
             ):
                 future = executor.submit(
                     func,
@@ -1278,7 +1261,7 @@ def _get_all_documents(
                 for key, entry in query_params.items()
                 if isinstance(entry, str)
                 and len(entry.split(",")) > 0
-                and key not in MAPIClientSettings().QUERY_NO_PARALLEL  # type: ignore
+                and key not in MAPI_CLIENT_SETTINGS.QUERY_NO_PARALLEL  # type: ignore
             ),
             key=lambda item: item[1],
             reverse=True,
@@ -1369,10 +1352,9 @@ class CoreRester(BaseRester):
     def __getattr__(self, v: str):
         if v in self._sub_resters:
             if self._sub_resters[v]._obj is None:
-
                 self._sub_resters[v](
                     api_key=self.api_key,
-                    endpoint=self.endpoint.split(self.suffix)[0],
+                    endpoint=self.base_endpoint,
                     include_user_agent=self._include_user_agent,
                     session=self.session,
                     use_document_model=self.use_document_model,

mp_api/client/core/settings.py

@@ -99,3 +99,6 @@ class MAPIClientSettings(BaseSettings):
     def _get_endpoint_from_env(cls, v: str | None) -> str:
         """Support setting endpoint via MP_API_ENDPOINT environment variable."""
         return v or os.environ.get("MP_API_ENDPOINT") or _DEFAULT_ENDPOINT
+
+
+MAPI_CLIENT_SETTINGS = MAPIClientSettings()

mp_api/client/core/utils.py

@@ -3,6 +3,7 @@
 import os
 from importlib import import_module
 from typing import TYPE_CHECKING, Literal
+from urllib.parse import urljoin
 
 import orjson
 from emmet.core import __version__ as _EMMET_CORE_VER
@@ -11,7 +12,7 @@
 from packaging.version import parse as parse_version
 
 from mp_api.client.core.exceptions import MPRestError
-from mp_api.client.core.settings import MAPIClientSettings
+from mp_api.client.core.settings import MAPI_CLIENT_SETTINGS
 
 if TYPE_CHECKING:
     from typing import Any
@@ -58,9 +59,9 @@ def validate_api_key(api_key: str | None = None) -> str:
     # SETTINGS tries to read API key from ~/.config/.pmgrc.yaml
     api_key = api_key or os.getenv("MP_API_KEY")
     if not api_key:
-        from pymatgen.core import SETTINGS
+        from pymatgen.core import SETTINGS as PMG_SETTINGS
 
-        api_key = SETTINGS.get("PMG_MAPI_KEY")
+        api_key = PMG_SETTINGS.get("PMG_MAPI_KEY")
 
     if not api_key or len(api_key) != 32:
         addendum = " Valid API keys are 32 characters." if api_key else ""
@@ -84,7 +85,7 @@ def validate_ids(id_list: list[str]) -> list[str]:
     Returns:
         id_list: Returns original ID list if everything is formatted correctly.
     """
-    if len(id_list) > MAPIClientSettings().MAX_LIST_LENGTH:
+    if len(id_list) > MAPI_CLIENT_SETTINGS.MAX_LIST_LENGTH:
         raise MPRestError(
             "List of material/molecule IDs provided is too long. Consider removing the ID filter to automatically pull"
             " data for all IDs and filter locally."
@@ -96,6 +97,32 @@ def validate_ids(id_list: list[str]) -> list[str]:
     return [str(validate_identifier(idx)) for idx in id_list]
 
 
+def validate_endpoint(endpoint: str | None, suffix: str | None = None) -> str:
+    """Validate an endpoint with optional suffix.
+
+    NB: does not modify the endpoint in place,
+    returns a new variable.
+
+    Parameters
+    -----------
+    endpoint : str or None (default)
+        A string representing the endpoint URL or the default
+        in `mp_api.client.core.settings`
+    suffix : str or None (default)
+        Optional suffix to append to the endpoint.
+
+    Returns:
+    -----------
+    str : the validated endpoint
+    """
+    new_endpoint = endpoint or MAPI_CLIENT_SETTINGS.ENDPOINT
+    if suffix:
+        new_endpoint = urljoin(new_endpoint, suffix)
+    if not new_endpoint.endswith("/"):
+        new_endpoint += "/"
+    return new_endpoint
+
+
 class LazyImport:
     """Lazily import and load an object.
 

mp_api/client/mprester.py

@@ -24,11 +24,12 @@
 
 from mp_api.client.core import BaseRester, MPRestError, MPRestWarning
 from mp_api.client.core._oxygen_evolution import OxygenEvolution
-from mp_api.client.core.settings import MAPIClientSettings
+from mp_api.client.core.settings import MAPI_CLIENT_SETTINGS
 from mp_api.client.core.utils import (
     LazyImport,
     load_json,
     validate_api_key,
+    validate_endpoint,
     validate_ids,
 )
 from mp_api.client.routes import GENERIC_RESTERS
@@ -42,7 +43,6 @@
     from pymatgen.entries.computed_entries import ComputedEntry
 
 _EMMET_SETTINGS = EmmetSettings()
-_MAPI_SETTINGS = MAPIClientSettings()
 DEFAULT_THERMOTYPE_CRITERIA = {"thermo_types": ["GGA_GGA+U"]}
 
 RESTER_LAYOUT = {
@@ -80,7 +80,7 @@ def __init__(
         use_document_model: bool = True,
         session: Session | None = None,
         headers: dict | None = None,
-        mute_progress_bars: bool = _MAPI_SETTINGS.MUTE_PROGRESS_BARS,
+        mute_progress_bars: bool = MAPI_CLIENT_SETTINGS.MUTE_PROGRESS_BARS,
         **kwargs,
     ):
         """Initialize the MPRester.
@@ -119,7 +119,7 @@ def __init__(
         """
         self.api_key = validate_api_key(api_key)
 
-        self.endpoint = endpoint or _MAPI_SETTINGS.ENDPOINT
+        self.endpoint = validate_endpoint(endpoint) or MAPI_CLIENT_SETTINGS.ENDPOINT
         if not self.endpoint.endswith("/"):
             self.endpoint += "/"
 
@@ -176,7 +176,7 @@ def __init__(
         emmet_version = MPRester.get_emmet_version(self.endpoint)
 
         if version.parse(emmet_version.base_version) < version.parse(
-            _MAPI_SETTINGS.MIN_EMMET_VERSION
+            MAPI_CLIENT_SETTINGS.MIN_EMMET_VERSION
         ):
             warnings.warn(
                 "The installed version of the mp-api client may not be compatible with the API server. "

mp_api/client/routes/molecules/__init__.py

@@ -6,7 +6,7 @@
     k: LazyImport(f"mp_api.client.routes.molecules.{k}.{v}")
     for k, v in (
         ("molecules", "MoleculeRester"),
-        ("jcser", "JcesrMoleculesRester"),
+        ("jcesr", "JcesrMoleculesRester"),
         ("summary", "MoleculesSummaryRester"),
     )
 }

mp_api/client/routes/molecules/jcesr.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import warnings
 from collections import defaultdict
 
 from emmet.core.molecules_jcesr import MoleculesDoc
@@ -14,6 +15,14 @@ class JcesrMoleculesRester(BaseRester):
     document_model = MoleculesDoc  # type: ignore
     primary_key = "task_id"
 
+    def __init__(self, **kwargs):
+        """Throw deprecation warning when JCESR client is initialized."""
+        warnings.warn(
+            "NOTE: You are accessing the unmaintained legacy molecules data, "
+            "please use MPRester.molecules.summary."
+        )
+        super().__init__(**kwargs)
+
     def search(
         self,
         task_ids: str | list[str] | None = None,

mp_api/client/routes/molecules/molecules.py

@@ -1,7 +1,5 @@
 from __future__ import annotations
 
-import warnings
-
 from emmet.core.mpid import MPculeID
 from emmet.core.qchem.molecule import MoleculeDoc
 from emmet.core.settings import EmmetSettings
@@ -196,13 +194,5 @@ class MoleculeRester(BaseMoleculeRester):
     suffix = "molecules/core"
     _sub_resters = MOLECULES_RESTERS
 
-    def __getattr__(self, v: str):
-        if "jcesr" in v:
-            warnings.warn(
-                "NOTE: You are accessing the unmaintained legacy molecules data, "
-                "please use MPRester.molecules.summary."
-            )
-        super().__getattr__(v)
-
     def __dir__(self):
         return dir(MoleculeRester) + self._sub_resters

tests/core/test_utils.py

@@ -85,9 +85,9 @@ def test_id_validation():
     from emmet.core.mpid import MPID, AlphaID
 
     from mp_api.client.core.utils import validate_ids
-    from mp_api.client.core.settings import MAPIClientSettings
+    from mp_api.client.core.settings import MAPI_CLIENT_SETTINGS
 
-    max_num_idxs = MAPIClientSettings().MAX_LIST_LENGTH
+    max_num_idxs = MAPI_CLIENT_SETTINGS.MAX_LIST_LENGTH
 
     with pytest.raises(MPRestError, match="too long"):
         _ = validate_ids([f"mp-{x}" for x in range(max_num_idxs + 1)])

tests/molecules/test_jcesr.py

@@ -48,3 +48,8 @@ def test_client(rester):
         custom_field_tests=custom_field_tests,
         sub_doc_fields=sub_doc_fields,
     )
+
+
+def test_warning():
+    with pytest.warns(UserWarning, match="unmaintained legacy molecules"):
+        JcesrMoleculesRester()