Enable more ruff rules including A, DTZ, PGH and more (#3871)

* enable ruff rule `PGH`

* revert change in lobster.inputs test

* fix some PathLike-related issues

* more PGH003 fixes

* more PGH003 fixes

* fix unit test

* more fixes for analysis module

* finish all PGH

* add more ruff rules

* fix some `DTZ` datetime errors

* fix `A001`

* fix `TRY004`

* fix more `TRY` errors

* tweak format

* tweak rule selections

* tweak pyproject.toml

* disable NPY rule for this PR

* revert datetime utc replacement for now

* pre-commit auto-fixes

* reapply DTZ005 fixes

* fix DTZ007

* fix PLR0124: compare with itself

* fix unit test

* suppress or fix `A002`

* fix errors that don't show locally

* replace `match.group(x)` with `match[x]`

* correct err code

* more group(x) changes

* relocate no planned ruff families

* reverse rule selection

* enable rule family `D` for `pymatgen/viz`

* restore assert x == x in __eq__ tests and ignore ruff PLR0124 in test files

* cif -> CIF in doc str

* snake_case var names

* revert deletion of test for __eq__

* replace `[0:x]` slicing with `[:x]`

* replace one more [0:x] slicing

* replace multi-line `dict()` with `{}`

* split xcfunc tests

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>

dev_scripts/chemenv/get_plane_permutations_optimized.py

@@ -236,7 +236,7 @@ def random_permutations_iterator(initial_permutation, n_permutations):
                 sym_measures = [c["symmetry_measure"] for c in csms]
                 prt1(string="Continuous symmetry measures", printing_volume=printing_volume)
                 prt1(string=sym_measures, printing_volume=printing_volume)
-                csms_with_recorded_permutation = []  # type: ignore
+                csms_with_recorded_permutation: list = []
                 explicit_permutations = []
                 for icsm, csm in enumerate(csms):
                     found = False
@@ -308,7 +308,6 @@ def random_permutations_iterator(initial_permutation, n_permutations):
                 all_planes_point_indices += algo.other_plane_points
 
             # Setup of the permutations to be used for this algorithm
-
             indices = list(range(cg.coordination_number))
             if permutations_setup_type == "all":
                 perms_iterator = itertools.permutations(indices)
@@ -400,7 +399,8 @@ def random_permutations_iterator(initial_permutation, n_permutations):
                     else:
                         perms_used[some_perm] = 1
                 tcurrent = time.process_time()
-                time_left = (n_permutations - idx_perm) * (tcurrent - t0) / idx_perm  # type: ignore
+                assert n_permutations is not None
+                time_left = (n_permutations - idx_perm) * (tcurrent - t0) / idx_perm
                 time_left = f"{time_left:.1f}"
                 idx_perm += 1
             print(

dev_scripts/update_pt_data.py

@@ -30,24 +30,24 @@ def parse_oxi_state():
     oxi_data = re.sub("[\n\r]", "", oxi_data)
     patt = re.compile("<tr>(.*?)</tr>", re.MULTILINE)
 
-    for m in patt.finditer(oxi_data):
-        line = m.group(1)
+    for match in patt.finditer(oxi_data):
+        line = match[1]
         line = re.sub("</td>", "", line)
         line = re.sub("(<td>)+", "<td>", line)
         line = re.sub("</*a[^>]*>", "", line)
         el = None
         oxi_states = []
         common_oxi = []
         for tok in re.split("<td>", line.strip()):
-            m2 = re.match(r"<b>([A-Z][a-z]*)</b>", tok)
-            if m2:
-                el = m2.group(1)
+            match2 = re.match(r"<b>([A-Z][a-z]*)</b>", tok)
+            if match2:
+                el = match2[1]
             else:
-                m3 = re.match(r"(<b>)*([\+\-]\d)(</b>)*", tok)
-                if m3:
-                    oxi_states += [int(m3.group(2))]
-                    if m3.group(1):
-                        common_oxi += [int(m3.group(2))]
+                match3 = re.match(r"(<b>)*([\+\-]\d)(</b>)*", tok)
+                if match3:
+                    oxi_states += [int(match3[2])]
+                    if match3[1]:
+                        common_oxi += [int(match3[2])]
         if el in data:
             del data[el]["Max oxidation state"]
             del data[el]["Min oxidation state"]
@@ -79,7 +79,7 @@ def parse_ionic_radii():
         ionic_radii = {}
         for tok_idx in range(3, len(tokens)):
             if match := re.match(r"^\s*([0-9\.]+)", tokens[tok_idx]):
-                ionic_radii[int(header[tok_idx])] = float(match.group(1))
+                ionic_radii[int(header[tok_idx])] = float(match[1])
 
         if el in data:
             data[el][f"Ionic_radii{suffix}"] = ionic_radii

pymatgen/alchemy/filters.py

@@ -179,7 +179,7 @@ def __init__(self, structure_matcher: dict | StructureMatcher | None = None, sym
         self.symprec = symprec
         self.structure_list: dict[str, list[Structure]] = defaultdict(list)
         if not isinstance(structure_matcher, (dict, StructureMatcher, type(None))):
-            raise ValueError(f"{structure_matcher=} must be a dict, StructureMatcher or None")
+            raise TypeError(f"{structure_matcher=} must be a dict, StructureMatcher or None")
         if isinstance(structure_matcher, dict):
             self.structure_matcher = StructureMatcher.from_dict(structure_matcher)
         else:

pymatgen/alchemy/materials.py

@@ -270,10 +270,10 @@ def from_cif_str(
         primitive: bool = True,
         occupancy_tolerance: float = 1.0,
     ) -> Self:
-        """Generate TransformedStructure from a cif string.
+        """Generate TransformedStructure from a CIF string.
 
         Args:
-            cif_string (str): Input cif string. Should contain only one
+            cif_string (str): Input CIF string. Should contain only one
                 structure. For CIFs containing multiple structures, please use
                 CifTransmuter.
             transformations (list[Transformation]): Sequence of transformations
@@ -302,7 +302,7 @@ def from_cif_str(
             source = "uploaded cif"
         source_info = {
             "source": source,
-            "datetime": str(datetime.datetime.now()),
+            "datetime": str(datetime.datetime.now(tz=datetime.timezone.utc)),
             "original_file": raw_str,
             "cif_data": cif_dict[cif_keys[0]],
         }
@@ -330,7 +330,7 @@ def from_poscar_str(
         struct = poscar.structure
         source_info = {
             "source": "POSCAR",
-            "datetime": str(datetime.datetime.now()),
+            "datetime": str(datetime.datetime.now(tz=datetime.timezone.utc)),
             "original_file": raw_str,
         }
         return cls(struct, transformations, history=[source_info])

pymatgen/alchemy/transmuters.py

@@ -121,10 +121,10 @@ def append_transformation(self, transformation, extend_collection=False, clear_r
                 describes whether the transformation altered the structure
         """
         if self.ncores and transformation.use_multiprocessing:
-            with Pool(self.ncores) as p:
+            with Pool(self.ncores) as pool:
                 # need to condense arguments into single tuple to use map
                 z = ((ts, transformation, extend_collection, clear_redo) for ts in self.transformed_structures)
-                trafo_new_structs = p.map(_apply_transformation, z, 1)
+                trafo_new_structs = pool.map(_apply_transformation, z, 1)
                 self.transformed_structures = []
                 for ts in trafo_new_structs:
                     self.transformed_structures.extend(ts)
@@ -223,19 +223,17 @@ def from_structures(cls, structures, transformations=None, extend_collection=0)
 
 
 class CifTransmuter(StandardTransmuter):
-    """Generate a Transmuter from a cif string, possibly containing multiple
-    structures.
-    """
+    """Generate a Transmuter from a CIF string, possibly containing multiple structures."""
 
     def __init__(self, cif_string, transformations=None, primitive=True, extend_collection=False):
-        """Generate a Transmuter from a cif string, possibly
+        """Generate a Transmuter from a CIF string, possibly
         containing multiple structures.
 
         Args:
-            cif_string: A string containing a cif or a series of CIFs
+            cif_string: A string containing a CIF or a series of CIFs
             transformations: New transformations to be applied to all
                 structures
-            primitive: Whether to generate the primitive cell from the cif.
+            primitive: Whether to generate the primitive cell from the CIF.
             extend_collection: Whether to use more than one output structure
                 from one-to-many transformations. extend_collection can be a
                 number, which determines the maximum branching for each
@@ -262,7 +260,7 @@ def from_filenames(cls, filenames, transformations=None, primitive=True, extend_
         containing multiple structures.
 
         Args:
-            filenames: List of strings of the cif files
+            filenames: List of strings of the CIF files
             transformations: New transformations to be applied to all
                 structures
             primitive: Same meaning as in __init__.

pymatgen/analysis/chemenv/connectivity/connected_components.py

@@ -483,7 +483,7 @@ def compute_periodicity_cycle_basis(self) -> None:
                     for current_delta in this_cycle_deltas:
                         this_cycle_deltas_new.append(current_delta + delta)
                 this_cycle_deltas = this_cycle_deltas_new
-            all_deltas.extend(this_cycle_deltas)  # type: ignore
+            all_deltas.extend(this_cycle_deltas)
             all_deltas = get_linearly_independent_vectors(all_deltas)
             if len(all_deltas) == 3:
                 return
@@ -501,7 +501,7 @@ def compute_periodicity_cycle_basis(self) -> None:
                     current_delta = get_delta(n1, n2, e1data)
                     delta = get_delta(n2, n1, e2data)
                     current_delta += delta
-                    all_deltas.append(current_delta)  # type: ignore
+                    all_deltas.append(current_delta)
             else:
                 raise ValueError("Should not be here ...")
             all_deltas = get_linearly_independent_vectors(all_deltas)
@@ -770,7 +770,7 @@ def _edgedictkey_to_edgekey(key):
             except ValueError:
                 return key
         else:
-            raise ValueError("Edge key in a dict of dicts representation of a graph should be either a str or an int.")
+            raise TypeError("Edge key in a dict of dicts representation of a graph should be either a str or an int.")
 
     @staticmethod
     def _retuplify_edgedata(edata):