Replace general raise Exception and add missing raise keyword (#3728

)

* suppress `Exception` with `contextlib.suppress`

* fix `NotImplementedError`

* avoid using general `Exception`

* fix typo

* Revert "suppress `Exception` with `contextlib.suppress`"

This reverts commit f555334.

* tweak error messages

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>

pymatgen/analysis/chemenv/coordination_environments/coordination_geometries.py

@@ -1189,7 +1189,8 @@ def is_a_valid_coordination_geometry(
                 return True
             except LookupError:
                 return True
-        raise Exception("Should not be here !")
+        # TODO give a more helpful error message that suggests possible reasons and solutions
+        raise RuntimeError("Should not be here!")
 
     def pretty_print(self, type="implemented_geometries", maxcn=8, additional_info=None):
         """

pymatgen/analysis/molecule_matcher.py

@@ -538,7 +538,7 @@ def uniform_labels(self, mol1, mol2):
             return None, None  # Topologically different
 
         if iequal_atom1 != iequal_atom2:
-            raise Exception("Design Error! Equivalent atoms are inconsistent")
+            raise RuntimeError("Design Error! Equivalent atoms are inconsistent")
 
         vmol1 = self._virtual_molecule(ob_mol1, ilabel1, iequal_atom1)
         vmol2 = self._virtual_molecule(ob_mol2, ilabel2, iequal_atom2)

pymatgen/apps/battery/battery_abc.py

@@ -227,7 +227,7 @@ def get_sub_electrodes(self, adjacent_only=True):
         Returns:
             A list of Electrode objects
         """
-        NotImplementedError(
+        raise NotImplementedError(
             "The get_sub_electrodes function must be implemented for each concrete electrode "
             f"class {type(self).__name__}"
         )

pymatgen/core/operations.py

@@ -484,9 +484,10 @@ def __init__(self, affine_transformation_matrix: ArrayLike, time_reversal: int,
             tol (float): Tolerance for determining if matrices are equal.
         """
         SymmOp.__init__(self, affine_transformation_matrix, tol=tol)
-        if time_reversal not in (-1, 1):
-            raise Exception(f"Time reversal operator not well defined: {time_reversal}, {type(time_reversal)}")
-        self.time_reversal = time_reversal
+        if time_reversal in {-1, 1}:
+            self.time_reversal = time_reversal
+        else:
+            raise RuntimeError(f"Invalid {time_reversal=}, must be 1 or -1")
 
     def __eq__(self, other: object) -> bool:
         if not isinstance(other, SymmOp):
@@ -592,7 +593,7 @@ def from_xyzt_str(cls, xyzt_str: str) -> Self:
         try:
             time_reversal = int(xyzt_str.rsplit(",", 1)[1])
         except Exception:
-            raise Exception("Time reversal operator could not be parsed.")
+            raise RuntimeError("Time reversal operator could not be parsed.")
         return cls.from_symmop(symm_op, time_reversal)
 
     def as_xyzt_str(self) -> str:

pymatgen/electronic_structure/bandstructure.py

@@ -214,7 +214,7 @@ def __init__(
             labels_dict = {}
 
         if len(self.projections) != 0 and self.structure is None:
-            raise Exception("if projections are provided a structure object needs also to be given")
+            raise RuntimeError("if projections are provided a structure object is also required")
 
         for k in kpoints:
             # let see if this kpoint has been assigned a label

pymatgen/io/adf.py

@@ -341,7 +341,7 @@ def is_float(s) -> bool:
         if string.find("subend") != -1:
             raise ValueError("Nested subkeys are not supported!")
 
-        def iterlines(s: str) -> Generator[str, None, None]:
+        def iter_lines(s: str) -> Generator[str, None, None]:
             r"""A generator form of s.split('\n') for reducing memory overhead.
 
             Args:
@@ -360,7 +360,7 @@ def iterlines(s: str) -> Generator[str, None, None]:
                 prev_nl = next_nl
 
         key = None
-        for line in iterlines(string):
+        for line in iter_lines(string):
             if line == "":
                 continue
             el = line.strip().split()
@@ -376,7 +376,7 @@ def iterlines(s: str) -> Generator[str, None, None]:
             elif key is not None:
                 key.add_subkey(cls.from_str(line))
 
-        raise Exception("IncompleteKey: 'END' is missing!")
+        raise KeyError("Incomplete key: 'END' is missing!")
 
 
 class AdfTask(MSONable):

pymatgen/io/cif.py

@@ -10,7 +10,7 @@
 from collections import defaultdict, deque
 from datetime import datetime
 from functools import partial
-from inspect import getfullargspec as getargspec
+from inspect import getfullargspec
 from io import StringIO
 from itertools import groupby
 from pathlib import Path
@@ -639,7 +639,7 @@ def get_lattice(
                 if data.data.get(lattice_label):
                     lattice_type = data.data.get(lattice_label).lower()
                     try:
-                        required_args = getargspec(getattr(Lattice, lattice_type)).args
+                        required_args = getfullargspec(getattr(Lattice, lattice_type)).args
 
                         lengths = (length for length in length_strings if length in required_args)
                         angles = (a for a in angle_strings if a in required_args)

pymatgen/io/vasp/outputs.py

@@ -974,10 +974,9 @@ def get_band_structure(
                     eigenvals[Spin.up] = up_eigen
             else:
                 if "" in kpoint_file.labels:
-                    raise Exception(
-                        "A band structure along symmetry lines "
-                        "requires a label for each kpoint. "
-                        "Check your KPOINTS file"
+                    raise ValueError(
+                        "A band structure along symmetry lines requires a label "
+                        "for each kpoint. Check your KPOINTS file"
                     )
                 labels_dict = dict(zip(kpoint_file.labels, kpoint_file.kpts))
                 labels_dict.pop(None, None)
@@ -2750,8 +2749,7 @@ def p_ion(results, match):
                 self.er_bp_tot = self.er_bp[Spin.up] + self.er_bp[Spin.down]
 
         except Exception:
-            self.er_ev_tot = self.er_bp_tot = None
-            raise Exception("IGPAR OUTCAR could not be parsed.")
+            raise RuntimeError("IGPAR OUTCAR could not be parsed.")
 
     def read_internal_strain_tensor(self):
         """
@@ -2780,7 +2778,7 @@ def internal_strain_data(results, match):
             elif match.group(1).lower() == "z":
                 index = 2
             else:
-                raise Exception(f"Couldn't parse row index from symbol for internal strain tensor: {match.group(1)}")
+                raise IndexError(f"Couldn't parse row index from symbol for internal strain tensor: {match.group(1)}")
             results.internal_strain_tensor[results.internal_strain_ion][index] = np.array(
                 [float(match.group(i)) for i in range(2, 8)]
             )
@@ -2952,7 +2950,7 @@ def born_section_stop(results, _match):
             self.piezo_tensor = self.piezo_tensor.tolist()
 
         except Exception:
-            raise Exception("LEPSILON OUTCAR could not be parsed.")
+            raise RuntimeError("LEPSILON OUTCAR could not be parsed.")
 
     def read_lepsilon_ionic(self):
         """
@@ -3069,7 +3067,7 @@ def piezo_section_stop(results, _match):
             self.piezo_ionic_tensor = self.piezo_ionic_tensor.tolist()
 
         except Exception:
-            raise Exception("ionic part of LEPSILON OUTCAR could not be parsed.")
+            raise RuntimeError("ionic part of LEPSILON OUTCAR could not be parsed.")
 
     def read_lcalcpol(self):
         """
@@ -3172,7 +3170,7 @@ def p_ion(results, match):
 
         except Exception as exc:
             print(exc.args)
-            raise Exception("LCALCPOL OUTCAR could not be parsed.") from exc
+            raise RuntimeError("LCALCPOL OUTCAR could not be parsed.") from exc
 
     def read_pseudo_zval(self):
         """Create pseudopotential ZVAL dictionary."""
@@ -3203,7 +3201,7 @@ def zvals(results, match):
             del self.atom_symbols
             del self.zvals
         except Exception:
-            raise Exception("ZVAL dict could not be parsed.")
+            raise RuntimeError("ZVAL dict could not be parsed.")
 
     def read_core_state_eigen(self):
         """
@@ -4042,9 +4040,9 @@ def __init__(self, filename, ionicstep_start=1, ionicstep_end=None, comment=None
         structures = []
         preamble_done = False
         if ionicstep_start < 1:
-            raise Exception("Start ionic step cannot be less than 1")
+            raise ValueError("Start ionic step cannot be less than 1")
         if ionicstep_end is not None and ionicstep_start < 1:
-            raise Exception("End ionic step cannot be less than 1")
+            raise ValueError("End ionic step cannot be less than 1")
 
         ionicstep_cnt = 1
         with zopen(filename, mode="rt") as file:
@@ -4136,9 +4134,9 @@ def concatenate(self, filename, ionicstep_start=1, ionicstep_end=None):
         structures = self.structures
         preamble_done = False
         if ionicstep_start < 1:
-            raise Exception("Start ionic step cannot be less than 1")
+            raise ValueError("Start ionic step cannot be less than 1")
         if ionicstep_end is not None and ionicstep_start < 1:
-            raise Exception("End ionic step cannot be less than 1")
+            raise ValueError("End ionic step cannot be less than 1")
 
         ionicstep_cnt = 1
         with zopen(filename, mode="rt") as file:
@@ -4187,9 +4185,9 @@ def get_str(self, ionicstep_start: int = 1, ionicstep_end: int | None = None, si
             significant_figures (int): Number of significant figures.
         """
         if ionicstep_start < 1:
-            raise Exception("Start ionic step cannot be less than 1")
+            raise ValueError("Start ionic step cannot be less than 1")
         if ionicstep_end is not None and ionicstep_end < 1:
-            raise Exception("End ionic step cannot be less than 1")
+            raise ValueError("End ionic step cannot be less than 1")
         lattice = self.structures[0].lattice
         if np.linalg.det(lattice.matrix) < 0:
             lattice = Lattice(-lattice.matrix)

pymatgen/transformations/advanced_transformations.py

@@ -772,7 +772,7 @@ def _remove_dummy_species(structure):
         merged with the original sites. Used after performing enumeration.
         """
         if not structure.is_ordered:
-            raise Exception("Something went wrong with enumeration.")
+            raise RuntimeError("Something went wrong with enumeration.")
 
         sites_to_remove = []
         logger.debug(f"Dummy species structure:\n{structure}")
@@ -787,7 +787,7 @@ def _remove_dummy_species(structure):
                     include_index=True,
                 )
                 if len(neighbors) != 1:
-                    raise Exception(f"This shouldn't happen, found neighbors: {neighbors}")
+                    raise RuntimeError(f"This shouldn't happen, found {neighbors=}")
                 orig_site_idx = neighbors[0][2]
                 orig_specie = structure[orig_site_idx].specie
                 new_specie = Species(