Fix S101, replace all assert in code base (except for tests) (#4017)

* turn on S101 for non test code

* replace some assert

* fix S101 in phonon

* fix bad replacement in util.coord

* fix S101 in dev_scripts

* fix alchemy apps.battery and command_line

* fix analysis

* fix entries

* fix core

* fix electronic_structure

* fix io.abinit and io.aims

* fix cif common and cp2k of io

* fix io gaussian packmol exciting and feff

* fix io.lammps

* fix io.vasp

* reapply ignore S101 in tests

* print variable values in err messages

* augment error message, thanks for the advice @janosh

* replace use of assert

* use type(x).__name__ in TypeError messages

fix typo

---------

Co-authored-by: Janosh Riebesell <[email protected]>

dev_scripts/chemenv/explicit_permutations_plane_algorithm.py

@@ -44,16 +44,14 @@
  raise ValueError("Should all be separation plane")
 
  perms_on_file = f"Permutations on file in this algorithm ({len(sep_plane_algo._permutations)}) "
- print(perms_on_file)
- print(sep_plane_algo._permutations)
+ print(f"{perms_on_file}\n{sep_plane_algo._permutations}")
  permutations = sep_plane_algo.safe_separation_permutations(
  ordered_plane=sep_plane_algo.ordered_plane, ordered_point_groups=sep_plane_algo.ordered_point_groups
  )
 
  sep_plane_algo._permutations = permutations
 
- print(f"Test permutations ({len(permutations)}) :")
- print(permutations)
+ print(f"Test permutations ({len(permutations)}):\n{permutations}")
 
  lgf = LocalGeometryFinder()
  lgf.setup_parameters(structure_refinement=lgf.STRUCTURE_REFINEMENT_NONE)

dev_scripts/chemenv/get_plane_permutations_optimized.py

@@ -398,9 +398,10 @@ def random_permutations_iterator(initial_permutation, n_permutations):
  perms_used[some_perm] += 1
  else:
  perms_used[some_perm] = 1
- tcurrent = time.process_time()
- assert n_permutations is not None
- time_left = (n_permutations - idx_perm) * (tcurrent - t0) / idx_perm
+ t_now = time.process_time()
+ if n_permutations is None:
+ raise ValueError(f"{n_permutations=}")
+ time_left = (n_permutations - idx_perm) * (t_now - t0) / idx_perm
  time_left = f"{time_left:.1f}"
  idx_perm += 1
  print(

dev_scripts/regen_libxcfunc.py

@@ -34,10 +34,12 @@ def parse_section(section):
  section += [line]
  else:
  num, entry = parse_section(section)
- assert num not in dct
+ if num in dct:
+ raise RuntimeError(f"{num=} should not be present in {dct=}.")
  dct[num] = entry
  section = []
- assert section == []
+ if section:
+ raise RuntimeError(f"Expected empty section, got {section=}")
 
  return dct
 

dev_scripts/update_pt_data.py

@@ -20,11 +20,11 @@
 except ImportError:
  BeautifulSoup = None
 
-ptable_yaml_path = "periodic_table.yaml"
+PTABLE_YAML_PATH = "periodic_table.yaml"
 
 
 def parse_oxi_state():
- data = loadfn(ptable_yaml_path)
+ data = loadfn(PTABLE_YAML_PATH)
  with open("oxidation_states.txt") as file:
  oxi_data = file.read()
  oxi_data = re.sub("[\n\r]", "", oxi_data)
@@ -62,7 +62,7 @@ def parse_oxi_state():
 
 
 def parse_ionic_radii():
- data = loadfn(ptable_yaml_path)
+ data = loadfn(PTABLE_YAML_PATH)
  with open("ionic_radii.csv") as file:
  radii_data = file.read()
  radii_data = radii_data.split("\r")
@@ -92,7 +92,7 @@ def parse_ionic_radii():
 
 
 def parse_radii():
- data = loadfn(ptable_yaml_path)
+ data = loadfn(PTABLE_YAML_PATH)
  with open("radii.csv") as file:
  radii_data = file.read()
  radii_data = radii_data.split("\r")
@@ -128,7 +128,7 @@ def parse_radii():
 
 
 def update_ionic_radii():
- data = loadfn(ptable_yaml_path)
+ data = loadfn(PTABLE_YAML_PATH)
 
  for dct in data.values():
  if "Ionic_radii" in dct:
@@ -147,7 +147,7 @@ def update_ionic_radii():
 
 
 def parse_shannon_radii():
- data = loadfn(ptable_yaml_path)
+ data = loadfn(PTABLE_YAML_PATH)
 
  from openpyxl import load_workbook
 
@@ -179,13 +179,13 @@ def parse_shannon_radii():
  if el in data:
  data[el]["Shannon radii"] = dict(radii[el])
 
- dumpfn(data, ptable_yaml_path)
+ dumpfn(data, PTABLE_YAML_PATH)
  with open("../pymatgen/core/periodic_table.json", mode="w") as file:
  json.dump(data, file)
 
 
 def gen_periodic_table():
- data = loadfn(ptable_yaml_path)
+ data = loadfn(PTABLE_YAML_PATH)
 
  with open("../pymatgen/core/periodic_table.json", mode="w") as file:
  json.dump(data, file)
@@ -226,7 +226,7 @@ def gen_iupac_ordering():
  for el in periodic_table:
  periodic_table[el].pop("IUPAC ordering", None)
 
- # now add iupac ordering
+ # now add IUPAC ordering
  for el in periodic_table:
  if "IUPAC ordering" in periodic_table[el]:
  # sanity check that we don't cover the same element twice
@@ -253,23 +253,26 @@ def add_electron_affinities():
  data += [row]
  data.pop(0)
 
- ea = {}
+ element_electron_affinities = {}
  max_Z = max(Element(element).Z for element in Element.__members__)
  for r in data:
  # don't want superheavy elements or less common isotopes
- if int(r[0]) > max_Z or r[2] in ea:
+ if int(r[0]) > max_Z or r[2] in element_electron_affinities:
  continue
  temp_str = re.sub(r"[\s\(\)]", "", r[3].strip("()[]"))
  # hyphen-like characters used that can't be parsed by .float
  bytes_rep = temp_str.encode("unicode_escape").replace(b"\\u2212", b"-")
- ea[r[2]] = float(bytes_rep.decode("unicode_escape"))
-
- Z_set = {Element.from_name(element).Z for element in ea}
- assert Z_set.issuperset(range(1, 93)) # Ensure that we have data for up to U.
- print(ea)
+ element_electron_affinities[r[2]] = float(bytes_rep.decode("unicode_escape"))
+
+ Z_set = {Element.from_name(element).Z for element in element_electron_affinities}
+ # Ensure that we have data for up to Uranium
+ if not Z_set.issuperset(range(1, 93)):
+ missing_electron_affinities = set(range(1, 93)) - Z_set
+ raise ValueError(f"{missing_electron_affinities=}")
+ print(element_electron_affinities)
  pt = loadfn("../pymatgen/core/periodic_table.json")
  for key, val in pt.items():
- val["Electron affinity"] = ea.get(Element(key).long_name)
+ val["Electron affinity"] = element_electron_affinities.get(Element(key).long_name)
  dumpfn(pt, "../pymatgen/core/periodic_table.json")
 
 
@@ -284,15 +287,17 @@ def add_ionization_energies():
  break
  data = defaultdict(list)
  for row in table.find_all("tr"):
- row = [td.get_text().strip() for td in row.find_all("td")]
- if row:
+ if row := [td.get_text().strip() for td in row.find_all("td")]:
  Z = int(row[0])
  val = re.sub(r"\s", "", row[8].strip("()[]"))
  val = None if val == "" else float(val)
  data[Z] += [val]
  print(data)
- print(data[51])
- assert set(data).issuperset(range(1, 93)) # Ensure that we have data for up to U.
+
+ # Ensure that we have data for up to U.
+ if not set(data).issuperset(range(1, 93)):
+ raise RuntimeError("Failed to get data up to Uranium")
+
  pt = loadfn("../pymatgen/core/periodic_table.json")
  for key, val in pt.items():
  del val["Ionization energy"]

pyproject.toml

@@ -220,7 +220,6 @@ ignore = [
  "PLR2004", # Magic-value-comparison TODO: fix these
  "PLW2901", # Outer for loop variable overwritten by inner assignment target
  "PT013", # Incorrect import of pytest
- "S101", # Use of "assert" TODO: fix these
  "S110", # Log for try-except-pass
  "S112", # Log for try-except-continue
  "S311", # Use random module for cryptographic purposes
@@ -248,7 +247,8 @@ docstring-code-format = true
  "PLR0124", # comparison-with-itself
  "PLR2004", # magic-value-comparison
  "PLR6301", # no-self-use
- ]
+ "S101", # Use of "assert"
+]
 "src/pymatgen/analysis/*" = ["D"]
 "src/pymatgen/io/*" = ["D"]
 "dev_scripts/*" = ["D"]

src/pymatgen/alchemy/transmuters.py

@@ -196,12 +196,12 @@ def append_transformed_structures(self, trafo_structs_or_transmuter):
  Args:
  trafo_structs_or_transmuter: A list of transformed structures or a transmuter.
  """
- if isinstance(trafo_structs_or_transmuter, self.__class__):
- self.transformed_structures += trafo_structs_or_transmuter.transformed_structures
- else:
- for ts in trafo_structs_or_transmuter:
- assert isinstance(ts, TransformedStructure)
-  self.transformed_structures += trafo_structs_or_transmuter
+ if not isinstance(trafo_structs_or_transmuter, self.__class__) and not all(
+ isinstance(ts, TransformedStructure) for ts in trafo_structs_or_transmuter
+ ):
+ raise TypeError("Some transformed structure has incorrect type.")
+
+ self.transformed_structures += trafo_structs_or_transmuter
 
  @classmethod
  def from_structures(cls, structures, transformations=None, extend_collection=0) -> Self:

src/pymatgen/analysis/diffraction/xrd.py

@@ -120,7 +120,8 @@ def __init__(self, wavelength="CuKa", symprec: float = 0, debye_waller_factors=N
  self.radiation = wavelength
  self.wavelength = WAVELENGTHS[wavelength]
  else:
- raise TypeError(f"{type(wavelength)=} must be either float, int or str")
+ wavelength_type = type(wavelength).__name__
+ raise TypeError(f"{wavelength_type=} must be either float, int or str")
  self.symprec = symprec
  self.debye_waller_factors = debye_waller_factors or {}
 

src/pymatgen/analysis/elasticity/elastic.py

@@ -82,7 +82,8 @@ def calculate_stress(self, strain):
  strain = np.array(strain)
  if strain.shape == (6,):
  strain = Strain.from_voigt(strain)
- assert strain.shape == (3, 3), "Strain must be 3x3 or Voigt notation"
+ if strain.shape != (3, 3):
+ raise ValueError(f"Strain must be 3x3 or Voigt notation, got {strain.shape=}")
  stress_matrix = self.einsum_sequence([strain] * (self.order - 1)) / factorial(self.order - 1)
  return Stress(stress_matrix)
 

src/pymatgen/analysis/phase_diagram.py

@@ -364,8 +364,10 @@ def __init__(
  computed_data = self._compute()
  else:
  computed_data = MontyDecoder().process_decoded(computed_data)
- assert isinstance(computed_data, dict)
- # update keys to be Element objects in case they are strings in pre-computed data
+ if not isinstance(computed_data, dict):
+ raise TypeError(f"computed_data should be dict, got {type(computed_data).__name__}")
+
+ # Update keys to be Element objects in case they are strings in pre-computed data
  computed_data["el_refs"] = [(Element(el_str), entry) for el_str, entry in computed_data["el_refs"]]
  self.computed_data = computed_data
  self.facets = computed_data["facets"]

src/pymatgen/analysis/structure_prediction/substitution_probability.py

@@ -150,7 +150,8 @@ def cond_prob_list(self, l1, l2):
  The conditional probability (assuming these species are in
  l2)
  """
- assert len(l1) == len(l2)
+ if len(l1) != len(l2):
+ raise ValueError("lengths of l1 and l2 mismatch.")
  p = 1
  for s1, s2 in zip(l1, l2, strict=True):
  p *= self.cond_prob(s1, s2)

src/pymatgen/analysis/structure_prediction/volume_predictor.py

@@ -204,7 +204,8 @@ def predict(self, structure: Structure, icsd_vol=False):
  if sp1 in bp_dict and sp2 in bp_dict:
  expected_dist = bp_dict[sp1] + bp_dict[sp2]
  else:
- assert sp1.atomic_radius is not None
+ if sp1.atomic_radius is None:
+ raise ValueError("atomic_radius of sp1 is None.")
  expected_dist = sp1.atomic_radius + sp2.atomic_radius
 
  if not smallest_ratio or nn.nn_distance / expected_dist < smallest_ratio:

src/pymatgen/apps/battery/conversion_battery.py

@@ -80,7 +80,8 @@ def from_composition_and_pd(
  if len(profile) < 2:
  return None
 
- assert working_ion_entry is not None
+ if working_ion_entry is None:
+ raise ValueError("working_ion_entry is None.")
  working_ion_symbol = working_ion_entry.elements[0].symbol
  normalization_els = {el: amt for el, amt in comp.items() if el != Element(working_ion_symbol)}
  framework = comp.as_dict()

src/pymatgen/command_line/gulp_caller.py

@@ -799,7 +799,8 @@ def __init__(self, bush_lewis_flag):
  Args:
  bush_lewis_flag (str): Flag for using Bush or Lewis potential.
  """
- assert bush_lewis_flag in {"bush", "lewis"}
+ if bush_lewis_flag not in {"bush", "lewis"}:
+ raise ValueError(f"bush_lewis_flag should be bush or lewis, got {bush_lewis_flag}")
  pot_file = "bush.lib" if bush_lewis_flag == "bush" else "lewis.lib"
  with open(os.path.join(os.environ["GULP_LIB"], pot_file)) as file:
  # In lewis.lib there is no shell for cation

src/pymatgen/core/interface.py

@@ -997,7 +997,8 @@ def get_trans_mat(
  c2_a2_ratio = 1.0 if ratio is None else ratio[0] / ratio[1]
  metric = np.array([[1, 0, 0], [0, 1, 0], [0, 0, c2_a2_ratio]])
  elif lat_type == "o":
- assert ratio is not None, "Invalid ratio for orthorhombic system"
+ if ratio is None:
+ raise ValueError(f"Invalid {ratio=} for orthorhombic system")
  for idx in range(3):
  if ratio is not None and ratio[idx] is None:
  ratio[idx] = 1
@@ -1203,9 +1204,13 @@ def get_trans_mat(
  mv = 1
 
  # Make sure mu, lambda, mv are coprime integers
- assert mu is not None
- assert lam is not None
- assert mv is not None
+ if mu is None:
+ raise ValueError("mu is None.")
+ if lam is None:
+ raise ValueError("lambda is None.")
+ if mv is None:
+ raise ValueError("mv is None.")
+
  if reduce(math.gcd, [mu, lam, mv]) != 1:
  temp = cast(int, reduce(math.gcd, [mu, lam, mv]))
  mu = round(mu / temp)
@@ -1253,7 +1258,8 @@ def get_trans_mat(
  raise RuntimeError("Sigma >1000 too large. Are you sure what you are doing, Please check the GB if exist")
  # Transform surface, r_axis, r_matrix in terms of primitive lattice
  surface = np.matmul(surface, np.transpose(trans_cry))
- assert surface is not None
+ if surface is None:
+ raise ValueError("surface is None.")
  fractions = [Fraction(x).limit_denominator() for x in surface]
  least_mul = reduce(lcm, [fraction.denominator for fraction in fractions])
  surface = cast(Tuple3Ints, tuple(round(x * least_mul) for x in surface))
@@ -1995,8 +2001,8 @@ def get_rotation_angle_from_sigma(
  lat_type = lat_type.lower()
 
  # Check r_axis length
- if lat_type in {"c", "t"}:
- assert len(r_axis) == 3, "r_axis length incompatible with selected lattice system"
+ if lat_type in {"c", "t"} and len(r_axis) != 3:
+ raise ValueError(f"expect r_axis length 3 for selected lattice system, got {len(r_axis)}")
 
  # Check lattice axial ratio length
  if lat_type == "o" and (ratio is None or len(ratio) != 3):
@@ -2484,9 +2490,8 @@ def __init__(
  vacuum_over_film: vacuum space above the film in Angstroms. Defaults to 0.
  interface_properties: properties associated with the Interface. Defaults to None.
  """
- assert (
- "interface_label" in site_properties
- ), "Must provide labeling of substrate and film sites in site properties"
+ if "interface_label" not in site_properties:
+ raise RuntimeError("Must provide labeling of substrate and film sites in site properties")
 
  self._in_plane_offset = np.array(in_plane_offset, dtype="float")
  self._gap = gap

src/pymatgen/core/operations.py

@@ -154,7 +154,8 @@ def transform_tensor(self, tensor: np.ndarray) -> np.ndarray:
  """
  dim = tensor.shape
  rank = len(dim)
- assert all(val == 3 for val in dim)
+ if any(val != 3 for val in dim):
+ raise ValueError("Some dimension in tensor is not 3.")
 
  # Build einstein sum string
  lc = string.ascii_lowercase

src/pymatgen/core/periodic_table.py

@@ -1318,7 +1318,8 @@ def get_shannon_radius(
  Shannon radius for specie in the specified environment.
  """
  radii = self._el.data["Shannon radii"]
- assert self._oxi_state is not None
+ if self._oxi_state is None:
+ raise ValueError("oxi_state is None.")
  radii = radii[str(int(self._oxi_state))][cn]
  if len(radii) == 1:
  key, data = next(iter(radii.items()))
@@ -1358,7 +1359,8 @@ def get_crystal_field_spin(
  if len(elec) < 4 or elec[-2][1] != "s" or elec[-1][1] != "d":
  raise AttributeError(f"Invalid element {self.symbol} for crystal field calculation")
 
- assert self.oxi_state is not None
+ if self.oxi_state is None:
+ raise ValueError("oxi_state is None.")
  n_electrons = elec[-1][2] + elec[-2][2] - self.oxi_state
  if n_electrons < 0 or n_electrons > 10:
  raise AttributeError(f"Invalid oxidation state {self.oxi_state} for element {self.symbol}")
@@ -1634,7 +1636,7 @@ def get_el_sp(obj: int | SpeciesLike) -> Element | Species | DummySpecies:
  # If obj is an integer, return the Element with atomic number obj
  try:
  flt = float(obj)
- assert flt == int(flt)
+ assert flt == int(flt) # noqa: S101
  return Element.from_Z(int(flt))
  except (AssertionError, ValueError, TypeError, KeyError):
  pass