IcohpValue.__str__ don't return None

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ ci:
 
 repos:
   - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: v0.0.270
+    rev: v0.0.271
     hooks:
       - id: ruff
         args: [--fix]

dev_scripts/chemenv/explicit_permutations.py

@@ -65,7 +65,7 @@ class Algo:
         coordination_geometry=cg, algo=algo, points_perfect=points_perfect
     )
 
-    csms_with_recorded_permutation = []
+    csms_with_recorded_permutation: list[float] = []
     explicit_permutations = []
     for icsm, csm in enumerate(csms):
         found = False

dev_scripts/chemenv/strategies/multi_weights_strategy_parameters.py

@@ -275,11 +275,11 @@ def get_weights(self, weights_options):
 
     with open("ce_pairs.json") as f:
         ce_pairs = json.load(f)
-    self_weight_max_csms = {}
-    self_weight_max_csms_per_cn = {}
-    allselfmaxcsms = []
-    delta_csm_mins = {}
-    alldeltacsmmins = []
+    self_weight_max_csms: dict[str, list[float]] = {}
+    self_weight_max_csms_per_cn: dict[str, list[float]] = {}
+    all_self_max_csms = []
+    delta_csm_mins: dict[str, list[float]] = {}
+    all_delta_csm_mins = []
     all_cn_pairs = []
     for ii in range(1, 14):
         self_weight_max_csms_per_cn[str(ii)] = []
@@ -293,28 +293,28 @@ def get_weights(self, weights_options):
         ce2 = ce_pair_dict["expected_final_environment_symbol"]
         cn_pair = f"{ce2.split(':')[1]}_{ce1.split(':')[1]}"
         nb_indices = ce_pair_dict["neighbors_indices"]
-        mindist = ce_pair_dict["dist_factor_min"]
-        maxdist = ce_pair_dict["dist_factor_max"]
+        min_dist = ce_pair_dict["dist_factor_min"]
+        max_dist = ce_pair_dict["dist_factor_max"]
         morph = CoordinationEnvironmentMorphing.simple_expansion(
             initial_environment_symbol=ce1, expected_final_environment_symbol=ce2, neighbors_indices=nb_indices
         )
-        params = morph.estimate_parameters(dist_factor_min=mindist, dist_factor_max=maxdist)
+        params = morph.estimate_parameters(dist_factor_min=min_dist, dist_factor_max=max_dist)
         print(f"For pair {ce1} to {ce2}, parameters are : ")
         print(params)
         self_weight_max_csms[cn_pair].append(params["self_weight_max_csm"])
         delta_csm_mins[cn_pair].append(params["delta_csm_min"])
-        allselfmaxcsms.append(params["self_weight_max_csm"])
-        alldeltacsmmins.append(params["delta_csm_min"])
+        all_self_max_csms.append(params["self_weight_max_csm"])
+        all_delta_csm_mins.append(params["delta_csm_min"])
         self_weight_max_csms_per_cn[ce1.split(":")[1]].append(params["self_weight_max_csm"])
 
     fig = plt.figure(1)
     subplot = fig.add_subplot(111)
 
-    for ipair, cn_pair in enumerate(all_cn_pairs):
+    for idx, cn_pair in enumerate(all_cn_pairs):
         if len(self_weight_max_csms[cn_pair]) == 0:
             continue
-        subplot.plot(ipair * np.ones_like(self_weight_max_csms[cn_pair]), self_weight_max_csms[cn_pair], "rx")
-        subplot.plot(ipair * np.ones_like(delta_csm_mins[cn_pair]), delta_csm_mins[cn_pair], "b+")
+        subplot.plot(idx * np.ones_like(self_weight_max_csms[cn_pair]), self_weight_max_csms[cn_pair], "rx")
+        subplot.plot(idx * np.ones_like(delta_csm_mins[cn_pair]), delta_csm_mins[cn_pair], "b+")
 
     subplot.set_xticks(range(len(all_cn_pairs)))
     subplot.set_xticklabels(all_cn_pairs, rotation="vertical")
@@ -330,16 +330,16 @@ def get_weights(self, weights_options):
 
     subplot2.set_xticks(range(1, 14))
     fig2.savefig("self_params_per_cn.pdf")
-    print(np.mean(allselfmaxcsms))
-    print(np.mean(alldeltacsmmins))
+    print(np.mean(all_self_max_csms))
+    print(np.mean(all_delta_csm_mins))
 
     fig3 = plt.figure(3, figsize=(24, 12))
     subplot3 = fig3.add_subplot(111)
 
-    for ipair, cn_pair in enumerate(all_cn_pairs):
+    for idx, cn_pair in enumerate(all_cn_pairs):
         if len(delta_csm_mins[cn_pair]) == 0:
             continue
-        subplot3.plot(ipair * np.ones_like(delta_csm_mins[cn_pair]), delta_csm_mins[cn_pair], "b+")
+        subplot3.plot(idx * np.ones_like(delta_csm_mins[cn_pair]), delta_csm_mins[cn_pair], "b+")
 
     subplot3.set_xticks(range(len(all_cn_pairs)))
     subplot3.set_xticklabels(all_cn_pairs, rotation="vertical")

pymatgen/analysis/bond_valence.py

@@ -340,7 +340,7 @@ def _recurse(assigned=None):
                 for sp, occu in get_z_ordered_elmap(sites[0].species):
                     elements.append(sp.symbol)
                     fractions.append(occu)
-            fractions = np.array(fractions, np.float_)
+            fractions = np.array(fractions, np.float_)  # type: ignore[assignment]
             new_valences = []
             for vals in valences:
                 for val in vals:

pymatgen/analysis/local_env.py

@@ -831,7 +831,7 @@ def get_all_voronoi_polyhedra(self, structure: Structure):
 
         # Get the non-duplicates (using the site indices for numerical stability)
         indices = np.array(indices, dtype=int)  # type: ignore
-        indices, uniq_inds = np.unique(indices, return_index=True, axis=0)
+        indices, uniq_inds = np.unique(indices, return_index=True, axis=0)  # type: ignore[assignment]
         sites = [sites[i] for i in uniq_inds]
 
         # Sort array such that atoms in the root image are first

pymatgen/core/tests/test_structure.py

@@ -1230,6 +1230,7 @@ def test_species(self):
         assert {*map(str, self.structure.species)} == {"Si"}
         assert len(self.structure.species) == len(self.structure)
 
+        # https://github.com/materialsproject/pymatgen/issues/3033
         with pytest.raises(AttributeError, match="species property only supports ordered structures!"):
             self.disordered.species
 

pymatgen/electronic_structure/cohp.py

@@ -868,98 +868,40 @@ def __init__(
         else:
             self._is_spin_polarized = False
 
-    def __str__(self):
+    def __str__(self) -> str:
+        """String representation of the ICOHP/ICOOP"""
         if not self._are_coops and not self._are_cobis:
             if self._is_spin_polarized:
                 return (
-                    "ICOHP "
-                    + str(self._label)
-                    + " between "
-                    + str(self._atom1)
-                    + " and "
-                    + str(self._atom2)
-                    + " ("
-                    + str(self._translation)
-                    + "): "
-                    + str(self._icohp[Spin.up])
-                    + " eV (Spin up) and "
-                    + str(self._icohp[Spin.down])
-                    + " eV (Spin down)"
+                    f"ICOHP {self._label} between {self._atom1} and {self._atom2} ({self._translation}): "
+                    f"{self._icohp[Spin.up]} eV (Spin up) and {self._icohp[Spin.down]} eV (Spin down)"
                 )
             return (
-                "ICOHP "
-                + str(self._label)
-                + " between "
-                + str(self._atom1)
-                + " and "
-                + str(self._atom2)
-                + " ("
-                + str(self._translation)
-                + "): "
-                + str(self._icohp[Spin.up])
-                + " eV (Spin up)"
+                f"ICOHP {self._label} between {self._atom1} and {self._atom2} ({self._translation}): "
+                f"{self._icohp[Spin.up]} eV (Spin up)"
             )
         if self._are_coops and not self._are_cobis:
             if self._is_spin_polarized:
                 return (
-                    "ICOOP "
-                    + str(self._label)
-                    + " between "
-                    + str(self._atom1)
-                    + " and "
-                    + str(self._atom2)
-                    + " ("
-                    + str(self._translation)
-                    + "): "
-                    + str(self._icohp[Spin.up])
-                    + " (Spin up) and "
-                    + str(self._icohp[Spin.down])
-                    + " (Spin down)"
+                    f"ICOOP {self._label} between {self._atom1} and {self._atom2} ({self._translation}): "
+                    f"{self._icohp[Spin.up]} eV (Spin up) and {self._icohp[Spin.down]} eV (Spin down)"
                 )
             return (
-                "ICOOP "
-                + str(self._label)
-                + " between "
-                + str(self._atom1)
-                + " and "
-                + str(self._atom2)
-                + " ("
-                + str(self._translation)
-                + "): "
-                + str(self._icohp[Spin.up])
-                + " (Spin up)"
+                f"ICOOP {self._label} between {self._atom1} and {self._atom2} ({self._translation}): "
+                f"{self._icohp[Spin.up]} eV (Spin up)"
             )
         if self._are_cobis and not self._are_coops:
             if self._is_spin_polarized:
                 return (
-                    "ICOBI "
-                    + str(self._label)
-                    + " between "
-                    + str(self._atom1)
-                    + " and "
-                    + str(self._atom2)
-                    + " ("
-                    + str(self._translation)
-                    + "): "
-                    + str(self._icohp[Spin.up])
-                    + " (Spin up) and "
-                    + str(self._icohp[Spin.down])
-                    + " (Spin down)"
+                    f"ICOBI {self._label} between {self._atom1} and {self._atom2} ({self._translation}): "
+                    f"{self._icohp[Spin.up]} eV (Spin up) and {self._icohp[Spin.down]} eV (Spin down)"
                 )
             return (
-                "ICOBI "
-                + str(self._label)
-                + " between "
-                + str(self._atom1)
-                + " and "
-                + str(self._atom2)
-                + " ("
-                + str(self._translation)
-                + "): "
-                + str(self._icohp[Spin.up])
-                + " (Spin up)"
+                f"ICOBI {self._label} between {self._atom1} and {self._atom2} ({self._translation}): "
+                f"{self._icohp[Spin.up]} eV (Spin up)"
             )
-        return None
+
+        return ""
 
     @property
     def num_bonds(self):

pymatgen/electronic_structure/tests/test_cohp.py

@@ -23,21 +23,21 @@
 
 class CohpTest(unittest.TestCase):
     def setUp(self):
-        with open(os.path.join(test_dir, "cohp.json")) as f:
-            self.cohp = Cohp.from_dict(json.load(f))
+        with open(os.path.join(test_dir, "cohp.json")) as file:
+            self.cohp = Cohp.from_dict(json.load(file))
         self.cohp_only = Cohp(self.cohp.efermi, self.cohp.energies, self.cohp.cohp)
-        with open(os.path.join(test_dir, "coop.json")) as f:
-            self.coop = Cohp.from_dict(json.load(f))
-        with open(os.path.join(test_dir, "cobi.json")) as f:
-            self.cobi = Cohp.from_dict(json.load(f))
+        with open(os.path.join(test_dir, "coop.json")) as file:
+            self.coop = Cohp.from_dict(json.load(file))
+        with open(os.path.join(test_dir, "cobi.json")) as file:
+            self.cobi = Cohp.from_dict(json.load(file))
 
     def test_as_from_dict(self):
-        with open(os.path.join(test_dir, "cohp.json")) as f:
-            cohp_dict = json.load(f)
+        with open(os.path.join(test_dir, "cohp.json")) as file:
+            cohp_dict = json.load(file)
         assert self.cohp.as_dict() == cohp_dict
 
-        with open(os.path.join(test_dir, "cobi.json")) as f:
-            cobi_dict = json.load(f)
+        with open(os.path.join(test_dir, "cobi.json")) as file:
+            cobi_dict = json.load(file)
         assert self.cobi.as_dict() == cobi_dict
 
     def test_attributes(self):
@@ -67,12 +67,17 @@ def test_get_interpolated_value(self):
             self.cohp_only.get_interpolated_value(5.0, integrated=True)
 
     def test_str(self):
-        with open(os.path.join(test_dir, "cohp.str")) as f:
-            str_cohp = f.read()
-        with open(os.path.join(test_dir, "coop.str")) as f:
-            str_coop = f.read()
+        header = "#Energy          COOPUp          ICOOPUp        \n"
+
+        with open(os.path.join(test_dir, "cohp.str")) as file:
+            str_cohp = file.read()
         assert str(self.cohp) == str_cohp
+        assert str(self.coop).strip().startswith(header)
+
+        with open(os.path.join(test_dir, "coop.str")) as file:
+            str_coop = file.read()
         assert str(self.coop) == str_coop
+        assert str(self.coop).strip().startswith(header)
 
     def test_antibnd_states_below_efermi(self):
         assert self.cohp.has_antibnd_states_below_efermi(spin=None) == {Spin.up: True, Spin.down: True}
@@ -149,6 +154,16 @@ def test_summed_icohp(self):
         # with spin polarization
         assert self.icohpvalue_sp.summed_icohp == -2.1
 
+    def test_str(self):
+        # without spin polarization
+        assert str(self.icohpvalue) == "ICOHP 1 between K1 and F2 ([-1, 0, 0]): -2.0 eV (Spin up)"
+
+        # with spin polarization
+        assert (
+            str(self.icohpvalue_sp)
+            == "ICOHP 1 between K1 and F2 ([-1, 0, 0]): -1.1 eV (Spin up) and -1.0 eV (Spin down)"
+        )
+
 
 class CombinedIcohpTest(unittest.TestCase):
     def setUp(self):