Test improvements (#3497)

janosh · web-flow · commit 65f4a6171388 · 2023-12-05T23:04:47.000Z
* update ruff-pre-commit to v0.1.7

* fic doc str indent

* improve test_get_neighbor_list

* improve test_ebands_input

* improve asserts in test_get_feff_pot

* remove unused assignment inside pytest.raises

* fix some unused var F841
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -8,7 +8,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.1.6
+    rev: v0.1.7
     hooks:
       - id: ruff
         args: [--fix]
diff --git a/pymatgen/analysis/adsorption.py b/pymatgen/analysis/adsorption.py
@@ -576,12 +576,12 @@ def substitute(site, i):
         # surface atoms, i.e. search for sites above (below) the bottom (top) surface
         sorted_sites = sorted(sym_slab, key=lambda site: site.frac_coords[2])
         if sorted_sites[0].surface_properties == "surface":
-            d = sorted_sites[0].frac_coords[2] + dist_from_surf
+            dist = sorted_sites[0].frac_coords[2] + dist_from_surf
         else:
-            d = sorted_sites[-1].frac_coords[2] - dist_from_surf
+            dist = sorted_sites[-1].frac_coords[2] - dist_from_surf
 
         for idx, site in enumerate(sym_slab):
-            if d - range_tol < site.frac_coords[2] < d + range_tol and (
+            if dist - range_tol < site.frac_coords[2] < dist + range_tol and (
                 target_species and site.species_string in target_species or not target_species
             ):
                 substituted_slabs.append(substitute(site, idx))
diff --git a/pymatgen/core/composition.py b/pymatgen/core/composition.py
@@ -363,7 +363,7 @@ def get_reduced_formula_and_factor(self, iupac_ordering: bool = False) -> tuple[
         if not all_int:
             return self.formula.replace(" ", ""), 1
         d = {k: int(round(v)) for k, v in self.get_el_amt_dict().items()}
-        (formula, factor) = reduce_formula(d, iupac_ordering=iupac_ordering)
+        formula, factor = reduce_formula(d, iupac_ordering=iupac_ordering)
 
         if formula in Composition.special_formulas:
             formula = Composition.special_formulas[formula]
diff --git a/pymatgen/electronic_structure/dos.py b/pymatgen/electronic_structure/dos.py
@@ -1097,7 +1097,10 @@ def get_dos_fp(
         n_bins: int = 256,
         normalize: bool = True,
     ) -> NamedTuple:
-        """Generates the DOS fingerprint based on work of
+        """Generates the DOS fingerprint.
+
+        Based on work of:
+
         F. Knoop, T. A. r Purcell, M. Scheffler, C. Carbogno, J. Open Source Softw. 2020, 5, 2671.
         Source - https://gitlab.com/vibes-developers/vibes/-/tree/master/vibes/materials_fp
         Copyright (c) 2020 Florian Knoop, Thomas A.R.Purcell, Matthias Scheffler, Christian Carbogno.
@@ -1117,7 +1120,7 @@ def get_dos_fp(
 
         Returns:
             Fingerprint(namedtuple) : The electronic density of states fingerprint
-            of format (energies, densities, type, n_bins)
+                of format (energies, densities, type, n_bins)
         """
         fingerprint = namedtuple("fingerprint", "energies densities type n_bins bin_width")
         energies = self.energies - self.efermi
diff --git a/pymatgen/symmetry/analyzer.py b/pymatgen/symmetry/analyzer.py
@@ -1155,7 +1155,7 @@ def not_on_axis(site):
             return np.linalg.norm(v) > self.tol
 
         valid_sets = []
-        origin_site, dist_el_sites = cluster_sites(self.centered_mol, self.tol)
+        _origin_site, dist_el_sites = cluster_sites(self.centered_mol, self.tol)
         for test_set in dist_el_sites.values():
             valid_set = list(filter(not_on_axis, test_set))
             if len(valid_set) > 0:
diff --git a/pymatgen/symmetry/kpath.py b/pymatgen/symmetry/kpath.py
@@ -2181,7 +2181,7 @@ def _get_max_cosine_labels(self, max_cosine_orbits_orig, key_points_inds_orbits,
         max_cosine_orbits_copy = max_cosine_orbits_orig.copy()
         max_cosine_label_inds = np.zeros(len(max_cosine_orbits_copy))
         initial_max_cosine_label_inds = [max_cos_orb[0][0] for max_cos_orb in max_cosine_orbits_copy]
-        u, inds, counts = np.unique(initial_max_cosine_label_inds, return_index=True, return_counts=True)
+        _uniq_vals, inds, counts = np.unique(initial_max_cosine_label_inds, return_index=True, return_counts=True)
         grouped_inds = [
             [
                 i
diff --git a/tests/analysis/solar/test_slme.py b/tests/analysis/solar/test_slme.py
@@ -14,3 +14,4 @@ def test_slme_from_vasprun(self):
         abz = abz * 100.0
         eff = slme(en, abz, indir_gap, indir_gap, plot_current_voltage=False)
         assert eff == approx(27.729, abs=1e-2)
+        assert dir_gap == approx(0.85389999, abs=1e-6)
diff --git a/tests/analysis/test_molecule_matcher.py b/tests/analysis/test_molecule_matcher.py
@@ -380,7 +380,7 @@ def test_mismatched_atom_composition(self):
         mol_matcher = HungarianOrderMatcher(mol1)
 
         with pytest.raises(ValueError, match="The number of the same species aren't matching"):
-            _, rmsd = mol_matcher.fit(mol2)
+            mol_matcher.fit(mol2)
 
     def test_fit(self):
         mol1 = Molecule.from_file(f"{test_dir}/benzene1.xyz")
@@ -485,7 +485,7 @@ def test_mismatched_atom_composition(self):
         mol_matcher = GeneticOrderMatcher(mol1, threshold=0.3)
 
         with pytest.raises(ValueError, match="The number of the same species aren't matching"):
-            _, rmsd = mol_matcher.fit(mol2)[0]
+            mol_matcher.fit(mol2)[0]
 
     def test_fit(self):
         mol1 = Molecule.from_file(f"{test_dir}/benzene1.xyz")
@@ -562,7 +562,7 @@ def test_mismatched_atoms(self):
         with pytest.raises(
             ValueError, match="The order of the species aren't matching! Please try using PermInvMatcher"
         ):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
     def test_rotated_molecule(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si_cluster_rotated.xyz")
@@ -600,7 +600,7 @@ def test_random_match(self):
             ValueError,
             match="The number of all possible permutations \\(20922789888000\\) is not feasible to run this method",
         ):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
 
 class TestHungarianOrderMatcherSi(unittest.TestCase):
@@ -617,7 +617,7 @@ def test_to_and_from_dict(self):
     def test_mismatched_atoms(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si2O_cluster_rotated.xyz")
         with pytest.raises(ValueError, match="The number of the same species aren't matching"):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
     def test_rotated_molecule(self):
         # TODO: Checking the cause of the large deviation
@@ -690,7 +690,7 @@ def test_mismatched_atoms(self):
         with pytest.raises(
             ValueError, match="The order of the species aren't matching! Please try using PermInvMatcher"
         ):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
     def test_rotated_molecule(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si2O_cluster_rotated.xyz")
@@ -709,7 +709,7 @@ def test_permuted_atoms_order(self):
         with pytest.raises(
             ValueError, match="The order of the species aren't matching! Please try using PermInvMatcher"
         ):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
 
 class TestBruteForceOrderMatcherSi2O(unittest.TestCase):
@@ -721,7 +721,7 @@ def setUpClass(cls):
     def test_mismatched_atoms(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si_cluster_rotated.xyz")
         with pytest.raises(ValueError, match="The number of the same species aren't matching"):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
     def test_rotated_molecule(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si2O_cluster_rotated.xyz")
@@ -753,7 +753,7 @@ def setUpClass(cls):
     def test_mismatched_atoms(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si_cluster_rotated.xyz")
         with pytest.raises(ValueError, match="The number of the same species aren't matching"):
-            _, rmsd = self.mol_matcher.fit(mol2)
+            self.mol_matcher.fit(mol2)
 
     def test_rotated_molecule(self):
         mol2 = Molecule.from_file(f"{test_dir}/Si2O_cluster_rotated.xyz")
diff --git a/tests/analysis/test_structure_matcher.py b/tests/analysis/test_structure_matcher.py
@@ -401,6 +401,7 @@ def test_find_match1(self):
         s2 = Structure(latt, ["Si", "Si", "Ag"], [[0, 0.1, 0], [0, 0.1, -0.95], [0.7, 0.5, 0.375]])
 
         s1, s2, fu, s1_supercell = sm._preprocess(s1, s2, niggli=False)
+        assert s1_supercell is True
         match = sm._strict_match(s1, s2, fu, s1_supercell=True, use_rms=True, break_on_match=False)
         scale_matrix = match[2]
         s2.make_supercell(scale_matrix)
diff --git a/tests/core/test_structure.py b/tests/core/test_structure.py
@@ -648,18 +648,17 @@ def test_get_all_neighbors_and_get_neighbors(self):
         assert len(neigh_sites) == 1
 
     def test_get_neighbor_list(self):
-        struct = self.struct
-        c_indices1, c_indices2, c_offsets, c_distances = struct.get_neighbor_list(3)
-        p_indices1, p_indices2, p_offsets, p_distances = struct._get_neighbor_list_py(3)
-        assert_allclose(sorted(c_distances), sorted(p_distances))
-
         # test mutable structure after applying strain (which makes lattice.matrix array no longer contiguous)
         # https://github.com/materialsproject/pymatgen/pull/3108
-        mutable_struct = Structure.from_sites(struct)
+        mutable_struct = Structure.from_sites(self.struct)
         mutable_struct.apply_strain(0.01)
-        c_indices1, c_indices2, c_offsets, c_distances = mutable_struct.get_neighbor_list(3)
-        p_indices1, p_indices2, p_offsets, p_distances = mutable_struct._get_neighbor_list_py(3)
-        assert_allclose(sorted(c_distances), sorted(p_distances))
+        for struct in (self.struct, mutable_struct):
+            cy_indices1, cy_indices2, cy_offsets, cy_distances = struct.get_neighbor_list(3)
+            py_indices1, py_indices2, py_offsets, py_distances = struct._get_neighbor_list_py(3)
+            assert_allclose(cy_distances, py_distances)
+            assert_allclose(cy_indices1, py_indices1)
+            assert_allclose(cy_indices2, py_indices2)
+            assert len(cy_offsets) == len(py_offsets)
 
     # @skipIf(not os.getenv("CI"), "Only run this in CI tests")
     # def test_get_all_neighbors_crosscheck_old(self):
@@ -728,6 +727,8 @@ def test_get_symmetric_neighbor_list(self):
         # tetragonal group with all bonds related by symmetry
         struct = Structure.from_spacegroup(100, [[1, 0, 0], [0, 1, 0], [0, 0, 2]], ["Fe"], [[0.0, 0.0, 0.0]])
         c_indices, p_indices, offsets, distances, s_indices, sym_ops = struct.get_symmetric_neighbor_list(0.8, sg=100)
+        assert len(c_indices) == len(p_indices) == len(offsets) == len(distances) == 8
+        assert c_indices == pytest.approx([0, 1, 1, 1, 0, 0, 0, 0])
         assert len(np.unique(s_indices)) == 1
         assert s_indices[0] == 0
         assert all(~np.isnan(s_indices))
@@ -756,6 +757,7 @@ def test_get_symmetric_neighbor_list(self):
         )
         assert all(np.sort(np.array([c_indices3, p_indices3]).flatten()) == np.sort(c_indices2))
         assert all(np.sort(np.array([c_indices3, p_indices3]).flatten()) == np.sort(p_indices2))
+        assert len(offsets3) == len(distances3) == len(s_indices3) == 24
 
     def test_get_all_neighbors_outside_cell(self):
         struct = Structure(
diff --git a/tests/io/abinit/test_inputs.py b/tests/io/abinit/test_inputs.py
@@ -281,9 +281,11 @@ def test_gs_input(self):
     def test_ebands_input(self):
         """Testing ebands_input factory."""
         multi = ebands_input(self.si_structure, self.si_pseudo, kppa=10, ecut=2)
-        str(multi)
+        assert str(multi).startswith("ndtset 2\n############")
 
         scf_inp, nscf_inp = multi.split_datasets()
+        assert isinstance(scf_inp, BasicAbinitInput)
+        assert isinstance(nscf_inp, BasicAbinitInput)
 
         # Test dos_kppa and other options.
         multi_dos = ebands_input(
@@ -311,7 +313,7 @@ def test_ebands_input(self):
             spin_mode="unpolarized",
             smearing=None,
             charge=2.0,
-            dos_kppa=[50, 100],
+            dos_kppa=(50, 100),
         )
         assert len(multi_dos) == 4
         assert multi_dos.get("iscf") == [None, -2, -2, -2]
diff --git a/tests/io/abinit/test_pseudos.py b/tests/io/abinit/test_pseudos.py
@@ -27,19 +27,19 @@ def setUp(self):
 
         self.nc_pseudos = collections.defaultdict(list)
 
-        for symbol, fnames in nc_pseudo_fnames.items():
-            for fname in fnames:
-                root, ext = os.path.splitext(fname)
-                pseudo = Pseudo.from_file(fname)
+        for symbol, file_names in nc_pseudo_fnames.items():
+            for file_name in file_names:
+                _root, ext = os.path.splitext(file_name)
+                pseudo = Pseudo.from_file(file_name)
                 self.nc_pseudos[symbol].append(pseudo)
 
                 # Save the pseudo as instance attribute whose name
                 # is constructed with the rule: symbol_ppformat
-                attr_name = symbol + "_" + ext[1:]
-                if hasattr(self, attr_name):
-                    raise RuntimeError(f"self has already the attribute {attr_name}")
+                attribute = f"{symbol}_{ext[1:]}"
+                if hasattr(self, attribute):
+                    raise RuntimeError(f"self has already {attribute=}")
 
-                setattr(self, attr_name, pseudo)
+                setattr(self, attribute, pseudo)
 
     def test_nc_pseudos(self):
         """Test norm-conserving pseudopotentials."""
diff --git a/tests/io/feff/test_inputs.py b/tests/io/feff/test_inputs.py
@@ -211,8 +211,9 @@ class TestFeffPot(unittest.TestCase):
     def test_init(self):
         filepath = f"{TEST_FILES_DIR}/POTENTIALS"
         feff_pot = Potential.pot_string_from_file(filepath)
-        d, dr = Potential.pot_dict_from_string(feff_pot)
-        assert d["Co"] == 1, "Wrong symbols read in for Potential"
+        dct, dr = Potential.pot_dict_from_string(feff_pot)
+        assert dct["Co"] == 1, "Wrong symbols read in for Potential"
+        assert dr == {0: "O", 1: "Co", 2: "O"}
 
     def test_single_absorbing_atom(self):
         """
diff --git a/tests/io/feff/test_sets.py b/tests/io/feff/test_sets.py
@@ -51,18 +51,19 @@ def test_get_feff_tags(self):
         assert tags["COREHOLE"] == "FSR", "Failed to generate PARAMETERS string"
 
     def test_get_feff_pot(self):
-        POT = str(self.mp_xanes.potential)
-        d, dr = Potential.pot_dict_from_string(POT)
-        assert d["Co"] == 1, "Wrong symbols read in for Potential"
+        potential = str(self.mp_xanes.potential)
+        dct, dr = Potential.pot_dict_from_string(potential)
+        assert dct["Co"] == 1, "Wrong symbols read in for Potential"
+        assert dr == {0: "O", 1: "Co", 2: "O"}
 
     def test_get_feff_atoms(self):
         atoms = str(self.mp_xanes.atoms)
         assert atoms.splitlines()[3].split()[4] == self.absorbing_atom, "failed to create ATOMS string"
 
     def test_to_and_from_dict(self):
-        f1_dict = self.mp_xanes.as_dict()
-        f2 = MPXANESSet.from_dict(f1_dict)
-        assert f1_dict == f2.as_dict()
+        dct = self.mp_xanes.as_dict()
+        xanes_set = MPXANESSet.from_dict(dct)
+        assert dct == xanes_set.as_dict(), "round trip as_dict failed"
 
     def test_user_tag_settings(self):
         tags_dict_ans = self.mp_xanes.tags.as_dict()
@@ -73,7 +74,7 @@ def test_user_tag_settings(self):
         assert mp_xanes_2.tags.as_dict() == tags_dict_ans
 
     def test_eels_to_from_dict(self):
-        elnes = MPELNESSet(
+        elnes_set = MPELNESSet(
             self.absorbing_atom,
             self.structure,
             radius=5.0,
@@ -82,7 +83,7 @@ def test_eels_to_from_dict(self):
             collection_angle=7,
             convergence_angle=6,
         )
-        elnes_dict = elnes.as_dict()
+        elnes_dict = elnes_set.as_dict()
         elnes_2 = MPELNESSet.from_dict(elnes_dict)
         assert elnes_dict == elnes_2.as_dict()
 
diff --git a/tests/io/test_res.py b/tests/io/test_res.py
@@ -54,13 +54,14 @@ def test_misc(self, provider: AirssProvider):
         assert date.day == 16
 
         castep_v = provider.get_castep_version()
-        assert castep_v is not None
         assert castep_v == "19.11"
 
         frd = provider.get_func_rel_disp()
         assert frd is not None
-        f, r, d = frd
-        assert f == "Perdew Burke Ernzerhof"
+        functional, rel, disp = frd
+        assert functional == "Perdew Burke Ernzerhof"
+        assert rel == "Koelling-Harmon"
+        assert disp == "off"
 
         airss_v = provider.get_airss_version()
         assert airss_v is not None

Original file line number	Diff line number	Diff line change
`@@ -2181,7 +2181,7 @@ def _get_max_cosine_labels(self, max_cosine_orbits_orig, key_points_inds_orbits,`
`2181`	`2181`	`max_cosine_orbits_copy = max_cosine_orbits_orig.copy()`
`2182`	`2182`	`max_cosine_label_inds = np.zeros(len(max_cosine_orbits_copy))`
`2183`	`2183`	`initial_max_cosine_label_inds = [max_cos_orb[0][0] for max_cos_orb in max_cosine_orbits_copy]`
`2184`		`- u, inds, counts = np.unique(initial_max_cosine_label_inds, return_index=True, return_counts=True)`
	`2184`	`+ _uniq_vals, inds, counts = np.unique(initial_max_cosine_label_inds, return_index=True, return_counts=True)`
`2185`	`2185`	`grouped_inds = [`
`2186`	`2186`	`[`
`2187`	`2187`	`i`