Merge branch 'master' into pr/chiang-yuan/3778

.coderabbit.yaml

@@ -0,0 +1,17 @@
+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+language: "en-US"
+early_access: false
+reviews:
+  request_changes_workflow: false
+  high_level_summary: false
+  poem: false
+  review_status: false
+  collapse_walkthrough: false
+  auto_review:
+    enabled: true
+    ignore_title_keywords:
+      - "WIP"
+      - "DO NOT MERGE"
+    drafts: false
+chat:
+  auto_reply: true

.github/workflows/lint.yml

@@ -22,7 +22,7 @@ jobs:
 
       - name: Install dependencies
         run: |
-          pip install --upgrade ruff mypy
+          pip install --upgrade ruff mypy pyright
 
       - name: ruff
         run: |
@@ -31,7 +31,7 @@ jobs:
           ruff format --check .
 
       - name: mypy
-        run: |
-          mypy --version
-          rm -rf .mypy_cache
-          mypy ${{ github.event.repository.name }}
+        run: mypy ${{ github.event.repository.name }}
+
+      - name: pyright
+        run: pyright

.pre-commit-config.yaml

@@ -2,13 +2,13 @@ exclude: ^(docs|tests/files|cmd_line|tasks.py)
 
 ci:
   autoupdate_schedule: monthly
-  skip: [mypy]
+  skip: [mypy, pyright]
   autofix_commit_msg: pre-commit auto-fixes
   autoupdate_commit_msg: pre-commit autoupdate
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.3.7
+    rev: v0.4.1
     hooks:
       - id: ruff
         args: [--fix, --unsafe-fixes]
@@ -35,7 +35,7 @@ repos:
         additional_dependencies: [tomli] # needed to read pyproject.toml below py3.11
 
   - repo: https://github.com/MarcoGorelli/cython-lint
-    rev: v0.16.0
+    rev: v0.16.2
     hooks:
       - id: cython-lint
         args: [--no-pycodestyle]
@@ -62,3 +62,8 @@ repos:
     hooks:
       - id: nbstripout
         args: [--drop-empty-cells, --keep-output]
+
+  - repo: https://github.com/RobertCraigie/pyright-python
+    rev: v1.1.359
+    hooks:
+      - id: pyright

dev_scripts/update_pt_data.py

@@ -234,7 +234,7 @@ def gen_iupac_ordering():
 def add_electron_affinities():
     """Update the periodic table data file with electron affinities."""
 
-    req = requests.get("https://wikipedia.org/wiki/Electron_affinity_(data_page)")
+    req = requests.get("https://wikipedia.org/wiki/Electron_affinity_(data_page)", timeout=600)
     soup = BeautifulSoup(req.text, "html.parser")
     table = None
     for table in soup.find_all("table"):

pymatgen/alchemy/filters.py

@@ -244,7 +244,7 @@ def __init__(self, existing_structures, structure_matcher=None, symprec=None):
                 structure matcher is used. A recommended value is 1e-5.
         """
         self.symprec = symprec
-        self.structure_list = []
+        self.structure_list: list = []
         self.existing_structures = existing_structures
         if isinstance(structure_matcher, dict):
             self.structure_matcher = StructureMatcher.from_dict(structure_matcher)

pymatgen/alchemy/transmuters.py

@@ -244,7 +244,7 @@ def __init__(self, cif_string, transformations=None, primitive=True, extend_coll
         """
         transformed_structures = []
         lines = cif_string.split("\n")
-        structure_data = []
+        structure_data: list = []
         read_data = False
         for line in lines:
             if re.match(r"^\s*data", line):

pymatgen/analysis/adsorption.py

@@ -680,19 +680,19 @@ def plot_slab(
         sites = list(reversed(sites))
         coords = np.array(reversed(coords))
     # Draw circles at sites and stack them accordingly
-    for n, coord in enumerate(coords):
-        radius = sites[n].species.elements[0].atomic_radius * scale
-        ax.add_patch(patches.Circle(coord[:2] - lattice_sum * (repeat // 2), radius, color="w", zorder=2 * n))
-        color = color_dict[sites[n].species.elements[0].symbol]
+    for idx, coord in enumerate(coords):
+        radius = sites[idx].species.elements[0].atomic_radius * scale
+        ax.add_patch(patches.Circle(coord[:2] - lattice_sum * (repeat // 2), radius, color="w", zorder=2 * idx))
+        color = color_dict[sites[idx].species.elements[0].symbol]
         ax.add_patch(
             patches.Circle(
                 coord[:2] - lattice_sum * (repeat // 2),
                 radius,
                 facecolor=color,
-                alpha=alphas[n],
+                alpha=alphas[idx],
                 edgecolor="k",
                 lw=0.3,
-                zorder=2 * n + 1,
+                zorder=2 * idx + 1,
             )
         )
     # Adsorption sites
@@ -714,7 +714,7 @@ def plot_slab(
         codes = [Path.MOVETO, Path.LINETO, Path.LINETO, Path.LINETO, Path.CLOSEPOLY]
         vertices = [(np.array(vert) + corner).tolist() for vert in vertices]
         path = Path(vertices, codes)
-        patch = patches.PathPatch(path, facecolor="none", lw=2, alpha=0.5, zorder=2 * n + 2)
+        patch = patches.PathPatch(path, facecolor="none", lw=2, alpha=0.5, zorder=2 * len(coords) + 2)
         ax.add_patch(patch)
     ax.set_aspect("equal")
     center = corner + lattice_sum / 2.0

pymatgen/analysis/bond_dissociation.py

@@ -118,8 +118,9 @@ def fragment_and_process(self, bonds):
             bonds (list): bonds to process.
         """
         # Try to split the principle:
+        fragments: list[MoleculeGraph] = []
         try:
-            frags = self.mol_graph.split_molecule_subgraphs(bonds, allow_reverse=True)
+            fragments = self.mol_graph.split_molecule_subgraphs(bonds, allow_reverse=True)
             frag_success = True
 
         except MolGraphSplitError:
@@ -166,7 +167,7 @@ def fragment_and_process(self, bonds):
                         # We shouldn't ever encounter more than one good entry.
                         raise RuntimeError("There should only be one valid ring opening fragment! Exiting...")
             elif len(bonds) == 2:
-                raise RuntimeError("Should only be trying to break two bonds if multibreak is true! Exiting...")
+                raise RuntimeError("Should only be trying to break two bonds if multibreak=True! Exiting...")
             else:
                 raise ValueError("No reason to try and break more than two bonds at once! Exiting...")
             frag_success = False
@@ -176,19 +177,19 @@ def fragment_and_process(self, bonds):
             # As above, we begin by making sure we haven't already encountered an identical pair of fragments:
             frags_done = False
             for frag_pair in self.done_frag_pairs:
-                if frag_pair[0].isomorphic_to(frags[0]):
-                    if frag_pair[1].isomorphic_to(frags[1]):
+                if frag_pair[0].isomorphic_to(fragments[0]):
+                    if frag_pair[1].isomorphic_to(fragments[1]):
                         frags_done = True
                         break
-                elif frag_pair[1].isomorphic_to(frags[0]) and frag_pair[0].isomorphic_to(frags[1]):
+                elif frag_pair[1].isomorphic_to(fragments[0]) and frag_pair[0].isomorphic_to(fragments[1]):
                     frags_done = True
                     break
             if not frags_done:
                 # If we haven't, we save this pair and search for the relevant fragment entries:
-                self.done_frag_pairs += [frags]
+                self.done_frag_pairs += [fragments]
                 n_entries_for_this_frag_pair = 0
-                frag1_entries = self.search_fragment_entries(frags[0])
-                frag2_entries = self.search_fragment_entries(frags[1])
+                frag1_entries = self.search_fragment_entries(fragments[0])
+                frag2_entries = self.search_fragment_entries(fragments[1])
                 frag1_charges_found = []
                 frag2_charges_found = []
                 # We then check for our expected charges of each fragment:
@@ -200,14 +201,14 @@ def fragment_and_process(self, bonds):
                         frag2_charges_found += [frag2["initial_molecule"]["charge"]]
                 # If we're missing some of either, tell the user:
                 if len(frag1_charges_found) < len(self.expected_charges):
-                    bb = BabelMolAdaptor(frags[0].molecule)
+                    bb = BabelMolAdaptor(fragments[0].molecule)
                     pb_mol = bb.pybel_mol
                     smiles = pb_mol.write("smi").split()[0]
                     for charge in self.expected_charges:
                         if charge not in frag1_charges_found:
                             warnings.warn(f"Missing {charge=} for fragment {smiles}")
                 if len(frag2_charges_found) < len(self.expected_charges):
-                    bb = BabelMolAdaptor(frags[1].molecule)
+                    bb = BabelMolAdaptor(fragments[1].molecule)
                     pb_mol = bb.pybel_mol
                     smiles = pb_mol.write("smi").split()[0]
                     for charge in self.expected_charges:

pymatgen/analysis/bond_valence.py

@@ -268,6 +268,7 @@ def get_valences(self, structure: Structure):
                 valences.append(vals)
 
         # make variables needed for recursion
+        attrib = []
         if structure.is_ordered:
             n_sites = np.array(list(map(len, equi_sites)))
             valence_min = np.array(list(map(min, valences)))
@@ -324,7 +325,6 @@ def _recurse(assigned=None):
         else:
             n_sites = np.array([len(sites) for sites in equi_sites])
             tmp = []
-            attrib = []
             for idx, n_site in enumerate(n_sites):
                 for _ in valences[idx]:
                     tmp.append(n_site)

pymatgen/analysis/chemenv/coordination_environments/chemenv_strategies.py

@@ -9,7 +9,7 @@
 
 import abc
 import os
-from typing import TYPE_CHECKING, ClassVar
+from typing import TYPE_CHECKING
 
 import numpy as np
 from monty.json import MSONable
@@ -31,6 +31,8 @@
 from pymatgen.symmetry.analyzer import SpacegroupAnalyzer
 
 if TYPE_CHECKING:
+    from typing import ClassVar
+
     from typing_extensions import Self
 
 __author__ = "David Waroquiers"
@@ -256,6 +258,7 @@ def equivalent_site_index_and_transform(self, psite):
             Equivalent site in the unit cell, translations and symmetry transformation.
         """
         # Get the index of the site in the unit cell of which the PeriodicSite psite is a replica.
+        isite = 0
         try:
             isite = self.structure_environments.structure.index(psite)
         except ValueError:
@@ -283,6 +286,8 @@ def equivalent_site_index_and_transform(self, psite):
         # that gets back the site to the unit cell (Translation III)
         # TODO: check that these tolerances are needed, now that the structures are refined before analyzing envs
         tolerances = [1e-8, 1e-7, 1e-6, 1e-5, 1e-4]
+        d_this_site2 = (0, 0, 0)
+        sym_trafo = None
         for tolerance in tolerances:
             for sym_op in self.symops:
                 new_site = PeriodicSite(

pymatgen/analysis/chemenv/coordination_environments/coordination_geometries.py

@@ -808,6 +808,9 @@ def edges(self, sites, permutation=None, input="sites"):
             coords = [site.coords for site in sites]
         elif input == "coords":
             coords = sites
+        else:
+            raise RuntimeError("Invalid input for edges.")
+
         # if permutation is None:
         #     coords = [site.coords for site in sites]
         # else:
@@ -908,7 +911,7 @@ def __init__(self, permutations_safe_override=False, only_symbols=None):
 
         self.minpoints = {}
         self.maxpoints = {}
-        self.separations_cg = {}
+        self.separations_cg: dict[int, dict] = {}
         for cn in range(6, 21):
             for cg in self.get_implemented_geometries(coordination=cn):
                 if only_symbols is not None and cg.ce_symbol not in only_symbols:

pymatgen/analysis/chemenv/coordination_environments/coordination_geometry_finder.py

@@ -775,6 +775,7 @@ def compute_structure_environments(
 
             to_add_from_hints = []
             nb_sets_info = {}
+            cn = 0
 
             for cn, nb_sets in struct_envs.neighbors_sets[isite].items():
                 if cn not in all_cns:
@@ -1534,8 +1535,7 @@ def coordination_geometry_symmetry_measures_separation_plane(
         algos = []
         perfect2local_maps = []
         local2perfect_maps = []
-        if testing:
-            separation_permutations = []
+        separation_permutations = []
         nplanes = 0
         for npoints in range(
             separation_plane_algo.minimum_number_of_points,
@@ -1793,9 +1793,10 @@ def _cg_csm_separation_plane(
         plane_found = False
         permutations = []
         permutations_symmetry_measures = []
-        if testing:
-            separation_permutations = []
+        separation_permutations = []
         dist_tolerances = dist_tolerances or DIST_TOLERANCES
+        algo = ""
+
         for dist_tolerance in dist_tolerances:
             algo = "NOT_FOUND"
             separation = local_plane.indices_separate(self.local_geometry._coords, dist_tolerance)

pymatgen/analysis/chemenv/coordination_environments/structure_environments.py

@@ -2085,7 +2085,7 @@ def __init__(self, coord_geoms=None):
             coord_geoms: coordination geometries to be added to the chemical environment.
         """
         if coord_geoms is None:
-            self.coord_geoms = {}
+            self.coord_geoms: dict = {}
         else:
             raise NotImplementedError(
                 "Constructor for ChemicalEnvironments with the coord_geoms argument is not yet implemented"
@@ -2230,6 +2230,7 @@ def __str__(self):
         if len(self.coord_geoms) == 0:
             out += " => No coordination in it <=\n"
             return out
+        mp_symbol = ""
         for key in self.coord_geoms:
             mp_symbol = key
             break

pymatgen/analysis/chemenv/utils/chemenv_config.py

@@ -78,12 +78,11 @@ def setup(self):
                 break
             elif test == "S":
                 config_file = self.save()
+                print(f"Configuration has been saved to file {config_file!r}")
                 break
             else:
                 print(" ... wrong key, try again ...")
             print()
-        if test == "S":
-            print(f"Configuration has been saved to file {config_file!r}")
 
     @property
     def has_materials_project_access(self):