Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .ci_support/environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ dependencies:
- sqsgenerator =0.5.4
- hatchling =1.29.0
- hatch-vcs =0.5.0
- mp-api =0.37.2
Comment thread
coderabbitai[bot] marked this conversation as resolved.
6 changes: 6 additions & 0 deletions src/structuretoolkit/build/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
from structuretoolkit.build.compound import B2, C14, C15, C36, D03
from structuretoolkit.build.mesh import create_mesh
from structuretoolkit.build.sqs import sqs_structures
from structuretoolkit.build.materialsproject import (
search as materialsproject_search,
by_id as materialsproject_by_id,
)
from structuretoolkit.build.surface import (
get_high_index_surface_info,
high_index_surface,
Expand All @@ -19,4 +23,6 @@
"sqs_structures",
"get_high_index_surface_info",
"high_index_surface",
"materialsproject_search",
"materialsproject_by_id",
]
139 changes: 139 additions & 0 deletions src/structuretoolkit/build/materialsproject.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
from typing import Any
from collections.abc import Generator
from ase.atoms import Atoms
from structuretoolkit.common.pymatgen import pymatgen_to_ase


def search(
chemsys: str | list[str], api_key=None, **kwargs
) -> Generator[dict[str, Any]]:
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
"""
Search the database for all structures matching the given query.

Note that `chemsys` takes distint values for unaries, binaries and so! A query with `chemsys=["Fe", "O"]` will
return iron structures and oxygen structures, but no iron oxide structures. Similarily `chemsys=["Fe-O"]` will
not return unary structures.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

All keyword arguments for filtering from the original API are supported. See the
`original docs <https://docs.materialsproject.org/downloading-data/using-the-api>`_ for them.

Search for all iron structures:

>>> pr = Project(...)
>>> irons = pr.create.structure.materialsproject.search("Fe")
>>> irons.number_of_structures
10

The returned :class:`~.MPQueryResults` object implements :class:`~.HasStructure` and can be accessed with the
material ids as a short-hand

>>> irons.get_structure(1) == irons.get_structure('mp-13')
True

Search for all structures with Al, Li that are on the T=0 convex hull:

>>> alli = pr.create.structure.materialsproject.search(['Al', 'Li', 'Al-Li'], is_stable=True)
>>> len(alli)
6
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Usage is only possible with an API key obtained from the Materials Project. To do this, create an account with
them, login and access `this webpage <https://next-gen.materialsproject.org/api#api-key>`.

Once you have a key, either pass it as the `api_key` parameter or export an
environment variable, called `MP_API_KEY`, in your shell setup.

Args:
chemsys (str, list of str): confine search to given elements; either an element symbol or multiple element
symbols seperated by dashes; if a list of strings is given return structures matching either of them
api_key (str, optional): if your API key is not exported in the environment flag MP_API_KEY, pass it here
**kwargs: passed verbatim to :meth:`mp_api.MPRester.summary.search` to further filter the results

Returns:
:class:`~.MPQueryResults`: resulting structures from the query
"""
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
from mp_api.client import MPRester

rest_kwargs = {
"use_document_model": False, # returns results as dictionaries
"include_user_agent": True, # send some additional software version info to MP
}
if api_key is not None:
rest_kwargs["api_key"] = api_key
with MPRester(**rest_kwargs) as mpr:
results = mpr.summary.search(
chemsys=chemsys, **kwargs, fields=["structure", "material_id"]
)
Comment on lines +61 to +65
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Prevent fields keyword collision in search() call.

At Line 63–65, callers can pass fields via **kwargs, which will conflict with the explicit fields=[...] and raise TypeError (“multiple values for keyword argument 'fields'”).

Proposed fix
-    with MPRester(**rest_kwargs) as mpr:
-        results = mpr.summary.search(
-            chemsys=chemsys, **kwargs, fields=["structure", "material_id"]
-        )
+    query_kwargs = dict(kwargs)
+    query_kwargs.pop("fields", None)
+    with MPRester(**rest_kwargs) as mpr:
+        results = mpr.summary.search(
+            chemsys=chemsys, fields=["structure", "material_id"], **query_kwargs
+        )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/structuretoolkit/build/materialsproject.py` around lines 63 - 65, The
search call passes explicit fields but also forwards **kwargs which may contain
a fields key causing a TypeError; before calling mpr.summary.search (the call
where chemsys and fields are passed), remove any 'fields' entry from kwargs
(e.g. kwargs.pop('fields', None)) or validate and raise a clear error if callers
pass their own fields, then call mpr.summary.search(chemsys=chemsys, **kwargs,
fields=[...]) so only the intended fields argument is provided.

for r in results:
if "structure" in r:
r["structure"] = pymatgen_to_ase(r["structure"])
yield r
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated


def by_id(
material_id: str | int,
final: bool = True,
conventional_unit_cell: bool = False,
api_key=None,
) -> Atoms | list[Atoms]:
"""
Retrieve a structure by material id.

This is how you would ask for the iron ground state:

>>> pr = Project(...)
>>> pr.create.structure.materialsproject.by_id('mp-13')
Fe: [0. 0. 0.]
tags:
spin: [(0: 2.214)]
pbc: [ True True True]
cell:
Cell([[2.318956, 0.000185, -0.819712], [-1.159251, 2.008215, -0.819524], [2.5e-05, 0.000273, 2.459206]])

Usage is only possible with an API key obtained from the Materials Project. To do this, create an account with
them, login and access `this webpage <https://next-gen.materialsproject.org/api#api-key>`.

Once you have a key, either pass it as the `api_key` parameter or export an
environment variable, called `MP_API_KEY`, in your shell setup.

Args:
material_id (str): the id assigned to a structure by the materials project
api_key (str, optional): if your API key is not exported in the environment flag MP_API_KEY, pass it here
final (bool, optional): if set to False, returns the list of initial structures,
else returns the final structure. (Default is True)
conventional_unit_cell (bool, optional): if set to True, returns the standard conventional unit cell.
(Default is False)

Returns:
:class:`~.Atoms`: requested final structure if final is True
list of :class:~.Atoms`: a list of initial (pre-relaxation) structures if final is False

Raises:
ValueError: material id does not exist
"""
from mp_api.client import MPRester

rest_kwargs = {
"include_user_agent": True, # send some additional software version info to MP
}
if api_key is not None:
rest_kwargs["api_key"] = api_key
with MPRester(**rest_kwargs) as mpr:
if final:
return pymatgen_to_ase(
mpr.get_structure_by_material_id(
material_id=material_id,
final=final,
conventional_unit_cell=conventional_unit_cell,
)
)
else:
return [
pymatgen_to_ase(mpr_structure)
for mpr_structure in (
mpr.get_structure_by_material_id(
material_id=material_id,
final=final,
conventional_unit_cell=conventional_unit_cell,
)
)
]
Loading