phonon collection is delta_backed, rename phonon_ids -> identifiers

- for internal consistency w/ server + data models

Author: tsmathis

mp_api/client/core/client.py

@@ -682,6 +682,7 @@ def _query_delta_backed(
         self,
         bucket: str,
         prefix: str,
+        access_controlled: bool = True,
         timeout: int | None = None,
         label: str | None = None,
     ) -> dict[str, Any]:
@@ -690,6 +691,7 @@ def _query_delta_backed(
         Args:
             bucket (str) : S3 OpenData bucket
             prefix (str) : S3 object prefix
+            access_controlled (bool): whether or not table has access controlled data
             timeout (int or None) : timeout on getting access-controlled groups
             label (str or None) : label of the table in QueryBuilder
 
@@ -756,7 +758,7 @@ def _query_delta_backed(
 
         predicate = (
             f"WHERE batch_id NOT IN ({controlled_batch_str})"
-            if not has_gnome_access and controlled_batch_str
+            if not has_gnome_access and controlled_batch_str and access_controlled
             else ""
         )
         # TODO: do we need something like this?
@@ -936,19 +938,19 @@ def _query_resource(
                 elif suffix in STATIC_COLLECTIONS:
                     bucket_suffix = "build"
                     prefix = f"static-collections/{suffix}"
-                elif self.delta_backed:
-                    return self._query_delta_backed(
-                        "materialsproject-build",
-                        f"collections/{suffix}/",
-                        timeout=timeout,
-                    )
                 else:
                     # TODO: remove once all collections are migrated to delta-backed format
                     bucket_suffix = "build"
-                    prefix = f"collections/{self.db_version.replace('.', '-')}/{suffix}"
+                    prefix = f"collections/{suffix}"
 
                 bucket = f"materialsproject-{bucket_suffix}"
 
+                if self.delta_backed:
+                    access_controlled = suffix not in STATIC_COLLECTIONS
+                    return self._query_delta_backed(
+                        bucket, prefix, access_controlled, timeout=timeout
+                    )
+
                 # Paginate over all entries in the bucket.
                 # TODO: change when a subset of entries needed from DB
                 paginator = self.s3_client.get_paginator("list_objects_v2")

mp_api/client/routes/materials/phonon.py

@@ -19,11 +19,10 @@ class PhononRester(BaseRester):
     suffix = "materials/phonon"
     document_model = PhononBSDOSDoc  # type: ignore
     primary_key = "identifier"
-    delta_backed = False
 
     def search(
         self,
-        phonon_ids: str | list[str] | None = None,
+        identifiers: str | list[str] | None = None,
         phonon_method: str | None = None,
         num_chunks: int | None = None,
         chunk_size: int = 1000,
@@ -34,7 +33,7 @@ def search(
         """Query phonon docs using a variety of search criteria.
 
         Arguments:
-            phonon_ids (str, List[str]): A single Phonon Task ID string or list of strings
+            identifiers (str, List[str]): A single Phonon Task ID string or list of strings
                 (e.g., mp-149, [mp-149, mp-13]).
             phonon_method (str): phonon method to search (dfpt, phonopy, pheasy)
             num_chunks (int): Maximum number of chunks of data to yield. None will yield all possible.
@@ -50,24 +49,24 @@ def search(
         query_params: dict = defaultdict(dict)
 
         if "material_ids" in kwargs:
-            if phonon_ids:
+            if identifiers:
                 raise MPRestError(
-                    "You have specified both `phonon_ids` and the deprecated `material_ids` tag. "
-                    "Please specify only `phonon_ids`."
+                    "You have specified both `identifiers` and the deprecated `material_ids` tag. "
+                    "Please specify only `identifiers`."
                 )
-            phonon_ids = kwargs.pop("material_ids")
+            identifiers = kwargs.pop("material_ids")
             warnings.warn(
                 "`material_id` has been replaced by `identifier` in the phonon endpoint. "
-                "Please migrate to using the newer field name and the more generic `phonon_ids` kwarg "
+                "Please migrate to using the newer field name and the more generic `identifiers` kwarg "
                 "for searching.",
                 stacklevel=2,
                 category=MPRestWarning,
             )
 
-        if phonon_ids:
-            query_params["phonon_ids"] = ",".join(
+        if identifiers:
+            query_params["identifiers"] = ",".join(
                 validate_ids(
-                    [phonon_ids] if isinstance(phonon_ids, str) else phonon_ids
+                    [identifiers] if isinstance(identifiers, str) else identifiers
                 )
             )
 
@@ -89,20 +88,20 @@ def search(
         )
 
     def get_bandstructure_from_phonon_id(
-        self, phonon_id: str, phonon_method: str
+        self, identifier: str, phonon_method: str
     ) -> PhononBS | dict[str, Any]:
         """Get the phonon band structure pymatgen object associated with a given phonon ID and phonon method.
 
         Arguments:
-            phonon_id (str): Phonon ID for the phonon band structure calculation
+            identifier (str): Phonon ID for the phonon band structure calculation
             phonon_method (str): phonon method, i.e. pheasy or dfpt
 
         Returns:
             bandstructure (PhononBS): PhononBS object
         """
         result = self._query_open_data(
             bucket="materialsproject-parsed",
-            key=f"ph-bandstructures/{phonon_method}/{phonon_id}.json.gz",
+            key=f"ph-bandstructures/{phonon_method}/{identifier}.json.gz",
         )[0][0]
 
         return (
@@ -117,27 +116,27 @@ def get_bandstructure_from_material_id(
         """Deprecated: use `get_bandstructure_from_phonon_id` instead."""
         warnings.warn(
             "`material_id` has been replaced by `identifier` in the phonon endpoint. "
-            "Please migrate to using `get_bandstructure_from_phonon_id` with the `phonon_id` kwarg.",
+            "Please migrate to using `get_bandstructure_from_phonon_id` with the `identifier` kwarg.",
             stacklevel=2,
             category=MPRestWarning,
         )
         return self.get_bandstructure_from_phonon_id(material_id, phonon_method)
 
     def get_dos_from_phonon_id(
-        self, phonon_id: str, phonon_method: str
+        self, identifier: str, phonon_method: str
     ) -> PhononDOS | dict[str, Any]:
         """Get the phonon dos pymatgen object associated with a given phonon ID and phonon method.
 
         Arguments:
-            phonon_id (str): Phonon ID for the phonon dos calculation
+            identifier (str): Phonon ID for the phonon dos calculation
             phonon_method (str): phonon method, i.e. pheasy or dfpt
 
         Returns:
             dos (PhononDOS): PhononDOS object
         """
         result = self._query_open_data(
             bucket="materialsproject-parsed",
-            key=f"ph-dos/{phonon_method}/{phonon_id}.json.gz",
+            key=f"ph-dos/{phonon_method}/{identifier}.json.gz",
         )[0][0]
 
         return (
@@ -152,24 +151,26 @@ def get_dos_from_material_id(
         """Deprecated: use `get_dos_from_phonon_id` instead."""
         warnings.warn(
             "`material_id` has been replaced by `identifier` in the phonon endpoint. "
-            "Please migrate to using `get_dos_from_phonon_id` with the `phonon_id` kwarg.",
+            "Please migrate to using `get_dos_from_phonon_id` with the `identifier` kwarg.",
             stacklevel=2,
             category=MPRestWarning,
         )
         return self.get_dos_from_phonon_id(material_id, phonon_method)
 
-    def get_forceconstants_from_phonon_id(self, phonon_id: str) -> list[list[Matrix3D]]:
+    def get_forceconstants_from_phonon_id(
+        self, identifier: str
+    ) -> list[list[Matrix3D]]:
         """Get the force constants associated with a given phonon ID.
 
         Arguments:
-            phonon_id (str): Phonon ID for the force constants calculation
+            identifier (str): Phonon ID for the force constants calculation
 
         Returns:
             force constants (list[list[Matrix3D]]): force constants
         """
         return self._query_open_data(  # type: ignore[return-value]
             bucket="materialsproject-parsed",
-            key=f"ph-force-constants/{phonon_id}.json.gz",
+            key=f"ph-force-constants/{identifier}.json.gz",
         )[0][0]
 
     def get_forceconstants_from_material_id(
@@ -178,54 +179,54 @@ def get_forceconstants_from_material_id(
         """Deprecated: use `get_forceconstants_from_phonon_id` instead."""
         warnings.warn(
             "`material_id` has been replaced by `identifier` in the phonon endpoint. "
-            "Please migrate to using `get_forceconstants_from_phonon_id` with the `phonon_id` kwarg.",
+            "Please migrate to using `get_forceconstants_from_phonon_id` with the `identifier` kwarg.",
             stacklevel=2,
             category=MPRestWarning,
         )
         return self.get_forceconstants_from_phonon_id(material_id)
 
     def compute_thermo_quantities(
         self,
-        phonon_id: str | None = None,
+        identifier: str | None = None,
         phonon_method: str | None = None,
         **kwargs,
     ):
         """Compute thermodynamical quantities for given phonon ID and phonon_method.
 
         Arguments:
-            phonon_id (str): Phonon ID to calculate quantities for
+            identifier (str): Phonon ID to calculate quantities for
             phonon_method (str): phonon method, i.e. pheasy or dfpt
             **kwargs : used for handling deprecated kwargs
 
         Returns:
             quantities (dict): thermodynamical quantities
         """
         if "material_id" in kwargs:
-            if phonon_id:
+            if identifier:
                 raise MPRestError(
-                    "You have specified both `phonon_id` and the deprecated `material_id` tag. "
-                    "Please specify only `phonon_id`."
+                    "You have specified both `identifier` and the deprecated `material_id` tag. "
+                    "Please specify only `identifier`."
                 )
-            phonon_id = kwargs.pop("material_id")
+            identifier = kwargs.pop("material_id")
             warnings.warn(
                 "`material_id` has been replaced by `identifier` in the phonon endpoint. "
-                "Please migrate to using the newer field name and the more generic `phonon_id` kwarg.",
+                "Please migrate to using the newer field name and the more generic `identifier` kwarg.",
                 stacklevel=2,
                 category=MPRestWarning,
             )
 
-        if phonon_id is None:
-            raise MPRestError("`phonon_id` must be specified.")
+        if identifier is None:
+            raise MPRestError("`identifier` must be specified.")
 
         use_document_model = self.use_document_model
         self.use_document_model = False
-        docs = self.search(phonon_ids=phonon_id, phonon_method=phonon_method)
+        docs = self.search(identifiers=identifier, phonon_method=phonon_method)
         if not docs or not docs[0]:
             raise MPRestError("No phonon document found")
 
         self.use_document_model = True
         docs[0]["phonon_dos"] = self.get_dos_from_phonon_id(  # type: ignore[index]
-            phonon_id, phonon_method  # type: ignore[arg-type]
+            identifier, phonon_method  # type: ignore[arg-type]
         )
         doc = PhononBSDOSDoc(**docs[0])  # type: ignore[arg-type]
         self.use_document_model = use_document_model