Merge remote-tracking branch 'upstream/master' into fstar

Author: jonathanjdenney

.github/workflows/lint.yml

@@ -31,7 +31,7 @@ jobs:
       - name: ruff
         run: |
           ruff --version
-          ruff . --ignore D
+          ruff .
 
       - name: black
         run: |

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.0.286
+    rev: v0.0.287
     hooks:
       - id: ruff
         args: [--fix]

dev_scripts/update_pt_data.py

@@ -78,16 +78,16 @@ def parse_ionic_radii():
     header = radii_data[0].split(",")
     for idx in range(1, len(radii_data)):
         line = radii_data[idx]
-        toks = line.strip().split(",")
+        tokens = line.strip().split(",")
         suffix = ""
-        name = toks[1]
+        name = tokens[1]
         if len(name.split(" ")) > 1:
             suffix = "_" + name.split(" ")[1]
-        el = toks[2]
+        el = tokens[2]
 
         ionic_radii = {}
-        for j in range(3, len(toks)):
-            m = re.match(r"^\s*([0-9\.]+)", toks[j])
+        for j in range(3, len(tokens)):
+            m = re.match(r"^\s*([0-9\.]+)", tokens[j])
             if m:
                 ionic_radii[int(header[j])] = float(m.group(1))
 
@@ -109,22 +109,22 @@ def parse_radii():
     radii_data = radii_data.split("\r")
 
     for line in radii_data:
-        toks = line.strip().split(",")
-        el = toks[1]
+        tokens = line.strip().split(",")
+        el = tokens[1]
         try:
-            atomic_radii = float(toks[3]) / 100
+            atomic_radii = float(tokens[3]) / 100
         except Exception:
-            atomic_radii = toks[3]
+            atomic_radii = tokens[3]
 
         try:
-            atomic_radii_calc = float(toks[4]) / 100
+            atomic_radii_calc = float(tokens[4]) / 100
         except Exception:
-            atomic_radii_calc = toks[4]
+            atomic_radii_calc = tokens[4]
 
         try:
-            vdw_radii = float(toks[5]) / 100
+            vdw_radii = float(tokens[5]) / 100
         except Exception:
-            vdw_radii = toks[5]
+            vdw_radii = tokens[5]
 
         if el in data:
             data[el]["Atomic radius"] = atomic_radii

pymatgen/alchemy/materials.py

@@ -11,6 +11,7 @@
 from typing import TYPE_CHECKING, Any
 from warnings import warn
 
+import numpy as np
 from monty.json import MSONable, jsanitize
 
 from pymatgen.core.structure import Structure
@@ -244,8 +245,14 @@ def structures(self) -> list[Structure]:
         h_structs = [Structure.from_dict(s["input_structure"]) for s in self.history if "input_structure" in s]
         return [*h_structs, self.final_structure]
 
-    @staticmethod
-    def from_cif_string(
+    @classmethod
+    @np.deprecate(message="Use from_cif_str instead")
+    def from_cif_string(cls, *args, **kwargs):  # noqa: D102
+        return cls.from_cif_str(*args, **kwargs)
+
+    @classmethod
+    def from_cif_str(
+        cls,
         cif_string: str,
         transformations: list[AbstractTransformation] | None = None,
         primitive: bool = True,
@@ -287,11 +294,16 @@ def from_cif_string(
             "original_file": raw_string,
             "cif_data": cif_dict[cif_keys[0]],
         }
-        return TransformedStructure(struct, transformations, history=[source_info])
+        return cls(struct, transformations, history=[source_info])
 
-    @staticmethod
-    def from_poscar_string(
-        poscar_string: str, transformations: list[AbstractTransformation] | None = None
+    @classmethod
+    @np.deprecate(message="Use from_poscar_str instead")
+    def from_poscar_string(cls, *args, **kwargs):  # noqa: D102
+        return cls.from_poscar_str(*args, **kwargs)
+
+    @classmethod
+    def from_poscar_str(
+        cls, poscar_string: str, transformations: list[AbstractTransformation] | None = None
     ) -> TransformedStructure:
         """Generates TransformedStructure from a poscar string.
 
@@ -300,29 +312,29 @@ def from_poscar_string(
             transformations (list[Transformation]): Sequence of transformations
                 to be applied to the input structure.
         """
-        p = Poscar.from_str(poscar_string)
-        if not p.true_names:
+        poscar = Poscar.from_str(poscar_string)
+        if not poscar.true_names:
             raise ValueError(
                 "Transformation can be created only from POSCAR strings with proper VASP5 element symbols."
             )
         raw_string = re.sub(r"'", '"', poscar_string)
-        struct = p.structure
+        struct = poscar.structure
         source_info = {
             "source": "POSCAR",
             "datetime": str(datetime.datetime.now()),
             "original_file": raw_string,
         }
-        return TransformedStructure(struct, transformations, history=[source_info])
+        return cls(struct, transformations, history=[source_info])
 
     def as_dict(self) -> dict[str, Any]:
         """Dict representation of the TransformedStructure."""
-        d = self.final_structure.as_dict()
-        d["@module"] = type(self).__module__
-        d["@class"] = type(self).__name__
-        d["history"] = jsanitize(self.history)
-        d["last_modified"] = str(datetime.datetime.utcnow())
-        d["other_parameters"] = jsanitize(self.other_parameters)
-        return d
+        dct = self.final_structure.as_dict()
+        dct["@module"] = type(self).__module__
+        dct["@class"] = type(self).__name__
+        dct["history"] = jsanitize(self.history)
+        dct["last_modified"] = str(datetime.datetime.utcnow())
+        dct["other_parameters"] = jsanitize(self.other_parameters)
+        return dct
 
     @classmethod
     def from_dict(cls, d) -> TransformedStructure:

pymatgen/alchemy/transmuters.py

@@ -117,9 +117,9 @@ def append_transformation(self, transformation, extend_collection=False, clear_r
             with Pool(self.ncores) as p:
                 # need to condense arguments into single tuple to use map
                 z = ((x, transformation, extend_collection, clear_redo) for x in self.transformed_structures)
-                new_tstructs = p.map(_apply_transformation, z, 1)
+                nrafo_ew_tstructs = p.map(_apply_transformation, z, 1)
                 self.transformed_structures = []
-                for ts in new_tstructs:
+                for ts in nrafo_ew_tstructs:
                     self.transformed_structures.extend(ts)
         else:
             new_structures = []
@@ -188,21 +188,21 @@ def __str__(self):
             output.append(str(x.final_structure))
         return "\n".join(output)
 
-    def append_transformed_structures(self, tstructs_or_transmuter):
+    def append_transformed_structures(self, trafo_structs_or_transmuter):
         """Method is overloaded to accept either a list of transformed structures
         or transmuter, it which case it appends the second transmuter"s
         structures.
 
         Args:
-            tstructs_or_transmuter: A list of transformed structures or a
+            trafo_structs_or_transmuter: A list of transformed structures or a
                 transmuter.
         """
-        if isinstance(tstructs_or_transmuter, self.__class__):
-            self.transformed_structures.extend(tstructs_or_transmuter.transformed_structures)
+        if isinstance(trafo_structs_or_transmuter, self.__class__):
+            self.transformed_structures.extend(trafo_structs_or_transmuter.transformed_structures)
         else:
-            for ts in tstructs_or_transmuter:
+            for ts in trafo_structs_or_transmuter:
                 assert isinstance(ts, TransformedStructure)
-            self.transformed_structures.extend(tstructs_or_transmuter)
+            self.transformed_structures.extend(trafo_structs_or_transmuter)
 
     @staticmethod
     def from_structures(structures, transformations=None, extend_collection=0):
@@ -221,8 +221,8 @@ def from_structures(structures, transformations=None, extend_collection=0):
         Returns:
             StandardTransmuter
         """
-        tstruct = [TransformedStructure(s, []) for s in structures]
-        return StandardTransmuter(tstruct, transformations, extend_collection)
+        trafo_struct = [TransformedStructure(s, []) for s in structures]
+        return StandardTransmuter(trafo_struct, transformations, extend_collection)
 
 
 class CifTransmuter(StandardTransmuter):
@@ -255,8 +255,8 @@ def __init__(self, cif_string, transformations=None, primitive=True, extend_coll
             if read_data:
                 structure_data[-1].append(line)
         for data in structure_data:
-            tstruct = TransformedStructure.from_cif_string("\n".join(data), [], primitive)
-            transformed_structures.append(tstruct)
+            trafo_struct = TransformedStructure.from_cif_string("\n".join(data), [], primitive)
+            transformed_structures.append(trafo_struct)
         super().__init__(transformed_structures, transformations, extend_collection)
 
     @staticmethod
@@ -295,8 +295,8 @@ def __init__(self, poscar_string, transformations=None, extend_collection=False)
             extend_collection: Whether to use more than one output structure
                 from one-to-many transformations.
         """
-        tstruct = TransformedStructure.from_poscar_string(poscar_string, [])
-        super().__init__([tstruct], transformations, extend_collection=extend_collection)
+        trafo_struct = TransformedStructure.from_poscar_string(poscar_string, [])
+        super().__init__([trafo_struct], transformations, extend_collection=extend_collection)
 
     @staticmethod
     def from_filenames(poscar_filenames, transformations=None, extend_collection=False):
@@ -310,11 +310,11 @@ def from_filenames(poscar_filenames, transformations=None, extend_collection=Fal
             extend_collection:
                 Same meaning as in __init__.
         """
-        tstructs = []
+        trafo_structs = []
         for filename in poscar_filenames:
             with open(filename) as f:
-                tstructs.append(TransformedStructure.from_poscar_string(f.read(), []))
-        return StandardTransmuter(tstructs, transformations, extend_collection=extend_collection)
+                trafo_structs.append(TransformedStructure.from_poscar_string(f.read(), []))
+        return StandardTransmuter(trafo_structs, transformations, extend_collection=extend_collection)
 
 
 def batch_write_vasp_input(

pymatgen/analysis/chemenv/connectivity/environment_nodes.py

@@ -42,7 +42,7 @@ def isite(self):
 
     def __hash__(self) -> int:
         """Simple hash function based on the hash function of the central site."""
-        return self.central_site.__hash__()
+        return hash(self.central_site)
 
     def __eq__(self, other: object) -> bool:
         if not isinstance(other, AbstractEnvironmentNode):

pymatgen/analysis/graphs.py

@@ -1151,8 +1151,8 @@ def __mul__(self, scaling_matrix):
         # this could probably be a lot smaller
         tol = 0.05
 
-        for u, v, k, d in new_g.edges(keys=True, data=True):
-            to_jimage = d["to_jimage"]  # for node v
+        for u, v, k, dct in new_g.edges(keys=True, data=True):
+            to_jimage = dct["to_jimage"]  # for node v
 
             # reduce unnecessary checking
             if to_jimage != (0, 0, 0):
@@ -1191,7 +1191,7 @@ def __mul__(self, scaling_matrix):
                 if v_present is not None:
                     new_u = u
                     new_v = v_present
-                    new_d = d.copy()
+                    new_d = dct.copy()
 
                     # node now inside supercell
                     new_d["to_jimage"] = (0, 0, 0)
@@ -1229,13 +1229,13 @@ def __mul__(self, scaling_matrix):
                     if v_present is not None:
                         new_u = u
                         new_v = v_present
-                        new_d = d.copy()
+                        new_d = dct.copy()
                         new_to_jimage = tuple(map(int, v_expec_image))
 
                         # normalize direction
                         if new_v < new_u:
                             new_u, new_v = new_v, new_u
-                            new_to_jimage = tuple(np.multiply(-1, d["to_jimage"]).astype(int))
+                            new_to_jimage = tuple(np.multiply(-1, dct["to_jimage"]).astype(int))
 
                         new_d["to_jimage"] = new_to_jimage
 
@@ -1250,18 +1250,18 @@ def __mul__(self, scaling_matrix):
         # add/delete marked edges
         for edge in edges_to_remove:
             new_g.remove_edge(*edge)
-        for u, v, d in edges_to_add:
-            new_g.add_edge(u, v, **d)
+        for u, v, dct in edges_to_add:
+            new_g.add_edge(u, v, **dct)
 
         # return new instance of StructureGraph with supercell
-        d = {
+        dct = {
             "@module": type(self).__module__,
             "@class": type(self).__name__,
             "structure": new_structure.as_dict(),
             "graphs": json_graph.adjacency_data(new_g),
         }
 
-        return StructureGraph.from_dict(d)
+        return StructureGraph.from_dict(dct)
 
     def __rmul__(self, other):
         return self.__mul__(other)
@@ -1307,7 +1307,7 @@ def __str__(self):
 
     def __repr__(self):
         s = "Structure Graph"
-        s += f"\nStructure: \n{self.structure.__repr__()}"
+        s += f"\nStructure: \n{self.structure!r}"
         s += f"\nGraph: {self.name}\n"
         s += self._edges_to_string(self.graph)
         return s
@@ -1363,8 +1363,7 @@ def __eq__(self, other: object) -> bool:
         if not isinstance(other, StructureGraph):
             return NotImplemented
         # sort for consistent node indices
-        # PeriodicSite should have a proper __hash__() value,
-        # using its frac_coords as a convenient key
+        # PeriodicSite should have a proper __hash__() value, using its frac_coords as a convenient key
         mapping = {tuple(site.frac_coords): self.structure.index(site) for site in other.structure}
         other_sorted = other.__copy__()
         other_sorted.sort(key=lambda site: mapping[tuple(site.frac_coords)])
@@ -1406,8 +1405,7 @@ def diff(self, other, strict=True):
 
         if strict:
             # sort for consistent node indices
-            # PeriodicSite should have a proper __hash__() value,
-            # using its frac_coords as a convenient key
+            # PeriodicSite should have a proper __hash__() value, using its frac_coords as a convenient key
             mapping = {tuple(site.frac_coords): self.structure.index(site) for site in other.structure}
             other_sorted = copy.copy(other)
             other_sorted.sort(key=lambda site: mapping[tuple(site.frac_coords)])
@@ -2668,7 +2666,7 @@ def __str__(self) -> str:
 
     def __repr__(self) -> str:
         out = "Molecule Graph"
-        out += f"\nMolecule: \n{self.molecule.__repr__()}"
+        out += f"\nMolecule: \n{self.molecule!r}"
         out += f"\nGraph: {self.name}\n"
         out += self._edges_to_string(self.graph)
         return out
@@ -2725,8 +2723,7 @@ def __eq__(self, other: object) -> bool:
             return NotImplemented
 
         # sort for consistent node indices
-        # PeriodicSite should have a proper __hash__() value,
-        # using its frac_coords as a convenient key
+        # PeriodicSite should have a proper __hash__() value, using its frac_coords as a convenient key
         try:
             mapping = {tuple(site.coords): self.molecule.index(site) for site in other.molecule}
         except ValueError:
@@ -2789,8 +2786,7 @@ def diff(self, other, strict=True):
 
         if strict:
             # sort for consistent node indices
-            # PeriodicSite should have a proper __hash__() value,
-            # using its frac_coords as a convenient key
+            # PeriodicSite should have a proper __hash__() value, using its frac_coords as a convenient key
             mapping = {tuple(site.frac_coords): self.molecule.index(site) for site in other.molecule}
             other_sorted = copy.copy(other)
             other_sorted.sort(key=lambda site: mapping[tuple(site.frac_coords)])

pymatgen/apps/battery/battery_abc.py

@@ -142,7 +142,7 @@ def __contains__(self, obj):
         return obj in self.voltage_pairs
 
     def __iter__(self):
-        return self.voltage_pairs.__iter__()
+        return iter(self.voltage_pairs)
 
     def __len__(self):
         return len(self.voltage_pairs)

pymatgen/cli/pmg_structure.py

@@ -54,9 +54,9 @@ def analyze_localenv(args):
     """
     bonds = {}
     for bond in args.localenv:
-        toks = bond.split("=")
-        species = toks[0].split("-")
-        bonds[(species[0], species[1])] = float(toks[1])
+        tokens = bond.split("=")
+        species = tokens[0].split("-")
+        bonds[(species[0], species[1])] = float(tokens[1])
     for filename in args.filenames:
         print(f"Analyzing {filename}...")
         data = []