Enable Ruff rule family "N" and "S" (#3892)

* enable ruff rule family N

* fix NPY002 in code (tests to be fixed)

* fix NPY002 in tests

* replace random with np where np is imported

* fix miller index type

* [Need Confirm] remove rnd seed and reduce distortion

* fix random seed

* correct replacement of randn with standard_normal

* enable rule family S and format tweaks

* suppress S501 error

* suppress S105 errors

* suppress S602

* replace weak `sha1` hash with `sha256`

* ignore S311 as pymatgen is not for cryptography

* suppress S605 for trusted source

* NEED CONFIRM: fix S607 for the starting of a process with a partial executable path

* suppress S310 for trusted source

* suppress S608 for trusted source

* NEED CONFIRMATION: rewrite subprocess without shell

* suppress S301, but still enable it because pickle is known to have potential security issues

* replace xml with defusedxml to fix S314

* Revert "replace xml with defusedxml to fix S314"

This reverts commit 4947ff5.

* ignore S314

* suppress S607 in tasks.py

* Revert "NEED CONFIRM: fix S607 for the starting of a process with a partial executable path"

This reverts commit 26ccb85.

* Fix DTZ003 deprecated datetime API

* pre-commit auto-fixes

* update monty to fix datetime serialization

* replace np.exceptions.RankWarning

* update monty in pyproject

* revert to np.RankWarning for now

* revert accidental np.trapz change during merge

* pre-commit auto-fixes

* pre-commit auto-fixes

* suppress S202 for trusted source

* ignore NPY201 for now

* Revert "pre-commit auto-fixes"

This reverts commit f6a7275.

* pre-commit auto-fixes

* fix indentation

* Revert "pre-commit auto-fixes"

This reverts commit 134a11c.

* regenerate requirement.txt

* fix indentation

* rename single-letter p = subprocess.run

* replace stdlib random with numpy

* avoid assign single use rng

* fix unit test

* keep non-imperative

* replace os.system with subprocess

* docstring and type tweaks for io.packmol

* io.packmol format tweaks

* fix subprocess run usage for stdin file

* add return type in docstring

* (feel free to revert) use list join over str concat

* use f-str

---------

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

dev_scripts/potcar_scrambler.py

@@ -48,20 +48,21 @@ def __init__(self, potcars: Potcar | PotcarSingle) -> None:
     def _rand_float_from_str_with_prec(self, input_str: str, bloat: float = 1.5) -> float:
         n_prec = len(input_str.split(".")[1])
         bd = max(1, bloat * abs(float(input_str)))  # ensure we don't get 0
-        return round(bd * np.random.rand(1)[0], n_prec)
+        return round(bd * np.random.default_rng().random(), n_prec)
 
     def _read_fortran_str_and_scramble(self, input_str: str, bloat: float = 1.5):
         input_str = input_str.strip()
+        rng = np.random.default_rng()
 
         if input_str.lower() in {"t", "f", "true", "false"}:
-            return bool(np.random.randint(2))
+            return rng.choice((True, False))
 
         if input_str.upper() == input_str.lower() and input_str[0].isnumeric():
             if "." in input_str:
                 return self._rand_float_from_str_with_prec(input_str, bloat=bloat)
             integer = int(input_str)
             fac = int(np.sign(integer))  # return int of same sign
-            return fac * np.random.randint(abs(max(1, int(np.ceil(bloat * integer)))))
+            return fac * rng.integers(abs(max(1, int(np.ceil(bloat * integer)))))
         try:
             float(input_str)
             return self._rand_float_from_str_with_prec(input_str, bloat=bloat)

pyproject.toml

@@ -188,12 +188,10 @@ ignore = [
     "ERA",  # Check for commented-out code
     "FIX",  # Check for FIXME, TODO and other developer notes
     "FURB", # refurb (need preview mode, too many preview errors)
-    "G",    # validate logging format strings
+    "G",    # Validate logging format strings
     "INP",  # Ban PEP-420 implicit namespace packages
-    "N",    # pep8-naming (many var/arg names are intended)
-    "NPY",  # NumPy-specific rules (TODO: enable this)
+    "N",    # PEP8-naming (many var/arg names are intended)
     "PTH",  # Prefer pathlib over os.path
-    "S",    # flake8-bandit (TODO: enable this)
     "SLF",  # Access "private" class members
     "T20",  # Check for print/pprint
     "TD",   # TODO tags related
@@ -204,24 +202,32 @@ ignore = [
     "B904",    # Within an except clause, raise exceptions with ...
     "C408",    # unnecessary-collection-call
     "D105",    # Missing docstring in magic method
-    "D205",    # 1 blank line required between summary line and description
+    "D205",    # One blank line required between summary line and description
     "D212",    # Multi-line docstring summary should start at the first line
     "FBT001",  # Boolean-typed positional argument in function definition
     "FBT002",  # Boolean default positional argument in function
+    "NPY201",  # TODO: enable after migration to NumPy 2.0
     "PD901",   # pandas-df-variable-name
-    "PERF203", # try-except-in-loop
-    "PERF401", # manual-list-comprehension
-    "PLR0911", # too many return statements
-    "PLR0912", # too many branches
-    "PLR0913", # too many arguments
-    "PLR0915", # too many statements
-    "PLR2004", # magic-value-comparison TODO fix these
+    "PERF203", # Use of try-except in for/while loop
+    "PERF401", # Replace "for" loops with list comprehension
+    "PLR0911", # Too many return statements
+    "PLR0912", # Too many branches
+    "PLR0913", # Too many arguments
+    "PLR0915", # Too many statements
+    "PLR2004", # Magic-value-comparison TODO fix these
     "PLW2901", # Outer for loop variable overwritten by inner assignment target
-    "PT013",   # pytest-incorrect-pytest-import
+    "PT013",   # Incorrect import of pytest
+    "S101",    # Use of "assert"
+    "S110",    # Log for try-except-pass
+    "S112",    # Log for try-except-continue
+    "S311",    # Use random module for cryptographic purposes
+    "S314",    # Replace xml with defusedxml to avoid XML attacks
+    "S603",    # Check source for use of "subprocess" call
+    "S607",    # Start process with relative path
     "SIM105",  # Use contextlib.suppress() instead of try-except-pass
     "TRY003",  # Avoid specifying long messages outside the exception class
-    "TRY300",  # Checks for return statements in try blocks
-    "TRY301",  # Checks for raise statements within try blocks
+    "TRY300",  # Check for return statements in try blocks
+    "TRY301",  # Check for raise statements within try blocks
 ]
 pydocstyle.convention = "google"
 isort.required-imports = ["from __future__ import annotations"]

requirements.txt

@@ -13,22 +13,17 @@ fonttools==4.53.0
 idna==3.7
     # via requests
 joblib==1.4.2
-    # via pymatgen (pyproject.toml)
 kiwisolver==1.4.5
     # via matplotlib
 latexcodec==3.0.0
     # via pybtex
 matplotlib==3.9.0
-    # via pymatgen (pyproject.toml)
-monty==2024.5.24
-    # via pymatgen (pyproject.toml)
+monty==2024.7.30
 mpmath==1.3.0
     # via sympy
 networkx==3.3
-    # via pymatgen (pyproject.toml)
 numpy==2.0.0
     # via
-    #   pymatgen (pyproject.toml)
     #   contourpy
     #   matplotlib
     #   pandas
@@ -39,15 +34,11 @@ packaging==24.1
     #   matplotlib
     #   plotly
 palettable==3.3.3
-    # via pymatgen (pyproject.toml)
 pandas==2.2.2
-    # via pymatgen (pyproject.toml)
 pillow==10.3.0
     # via matplotlib
 plotly==5.22.0
-    # via pymatgen (pyproject.toml)
 pybtex==0.24.0
-    # via pymatgen (pyproject.toml)
 pyparsing==3.1.2
     # via matplotlib
 python-dateutil==2.9.0.post0
@@ -59,30 +50,22 @@ pytz==2024.1
 pyyaml==6.0.1
     # via pybtex
 requests==2.32.3
-    # via pymatgen (pyproject.toml)
 ruamel-yaml==0.18.6
-    # via pymatgen (pyproject.toml)
 ruamel-yaml-clib==0.2.8
     # via ruamel-yaml
 scipy==1.13.1
-    # via pymatgen (pyproject.toml)
 six==1.16.0
     # via
     #   pybtex
     #   python-dateutil
-spglib==2.4.0
-    # via pymatgen (pyproject.toml)
+spglib==2.5.0
 sympy==1.12.1
-    # via pymatgen (pyproject.toml)
 tabulate==0.9.0
-    # via pymatgen (pyproject.toml)
 tenacity==8.4.1
     # via plotly
 tqdm==4.66.4
-    # via pymatgen (pyproject.toml)
 tzdata==2024.1
     # via pandas
 uncertainties==3.2.1
-    # via pymatgen (pyproject.toml)
 urllib3==2.2.2
     # via requests

src/pymatgen/analysis/chemenv/coordination_environments/coordination_geometry_finder.py

@@ -993,15 +993,17 @@ def setup_test_perfect_environment(
             raise ValueError("Wrong mp_symbol to setup coordination geometry")
         neighb_coords = []
         _points = points if points is not None else cg.points
+        rng = np.random.default_rng()
+
         if randomness:
-            rv = np.random.random_sample(3)
+            rv = rng.random(3)
             while norm(rv) > 1.0:
-                rv = np.random.random_sample(3)
+                rv = rng.random(3)
             coords = [np.zeros(3, float) + max_random_dist * rv]
             for pp in _points:
-                rv = np.random.random_sample(3)
+                rv = rng.random(3)
                 while norm(rv) > 1.0:
-                    rv = np.random.random_sample(3)
+                    rv = rng.random(3)
                 neighb_coords.append(np.array(pp) + max_random_dist * rv)
         else:
             coords = [np.zeros(3, float)]
@@ -1016,7 +1018,7 @@ def setup_test_perfect_environment(
 
         # Scaling the test environment
         if random_scale == "RANDOM":
-            scale = 0.1 * np.random.random_sample() + 0.95
+            scale = 0.1 * rng.random() + 0.95
         elif random_scale == "NONE":
             scale = 1.0
         else:
@@ -1026,9 +1028,9 @@ def setup_test_perfect_environment(
 
         # Rotating the test environment
         if random_rotation == "RANDOM":
-            uu = np.random.random_sample(3) + 0.1
+            uu = rng.random(3) + 0.1
             uu = uu / norm(uu)
-            theta = np.pi * np.random.random_sample()
+            theta = np.pi * rng.random()
             cos_theta = np.cos(theta)
             sin_theta = np.sin(theta)
             ux = uu[0]
@@ -1068,7 +1070,7 @@ def setup_test_perfect_environment(
 
         # Translating the test environment
         if random_translation == "RANDOM":
-            translation = 10.0 * (2.0 * np.random.random_sample(3) - 1.0)
+            translation = 10.0 * (2.0 * rng.random(3) - 1.0)
         elif random_translation == "NONE":
             translation = np.zeros(3, float)
         else:
@@ -1113,7 +1115,7 @@ def setup_random_structure(self, coordination):
         bb = -0.2
         coords = []
         for _ in range(coordination + 1):
-            coords.append(aa * np.random.random_sample(3) + bb)
+            coords.append(aa * np.random.default_rng().random(3) + bb)
         self.set_structure(
             lattice=np.array(np.eye(3) * 10, float),
             species=["Si"] * (coordination + 1),
@@ -1130,7 +1132,7 @@ def setup_random_indices_local_geometry(self, coordination):
         """
         self.icentral_site = 0
         self.indices = list(range(1, coordination + 1))
-        np.random.shuffle(self.indices)
+        np.random.default_rng().shuffle(self.indices)
 
     def setup_ordered_indices_local_geometry(self, coordination):
         """Set up ordered indices for the local geometry, for testing purposes.
@@ -2049,8 +2051,9 @@ def coordination_geometry_symmetry_measures_fallback_random(
         algos = []
         perfect2local_maps = []
         local2perfect_maps = []
+        rng = np.random.default_rng()
         for idx in range(n_random):
-            perm = np.random.permutation(coordination_geometry.coordination_number)
+            perm = rng.permutation(coordination_geometry.coordination_number)
             permutations.append(perm)
             p2l = {}
             l2p = {}

src/pymatgen/analysis/piezo_sensitivity.py

@@ -122,7 +122,7 @@ def get_rand_BEC(self, max_charge=1):
         BEC = np.zeros((n_atoms, 3, 3))
         for atom, ops in enumerate(self.BEC_operations):
             if ops[0] == ops[1]:
-                temp_tensor = Tensor(np.random.rand(3, 3) - 0.5)
+                temp_tensor = Tensor(np.random.default_rng().random((3, 3)) - 0.5)
                 temp_tensor = sum(temp_tensor.transform(symm_op) for symm_op in self.pointops[atom]) / len(
                     self.pointops[atom]
                 )
@@ -238,7 +238,7 @@ def get_rand_IST(self, max_force=1):
                 temp_tensor += op[1].transform_tensor(IST[op[0]])
 
             if len(ops) == 0:
-                temp_tensor = Tensor(np.random.rand(3, 3, 3) - 0.5)
+                temp_tensor = Tensor(np.random.default_rng().random((3, 3, 3)) - 0.5)
                 for dim in range(3):
                     temp_tensor[dim] = (temp_tensor[dim] + temp_tensor[dim].T) / 2
                 temp_tensor = sum(temp_tensor.transform(symm_op) for symm_op in self.pointops[atom]) / len(
@@ -385,7 +385,7 @@ def get_unstable_FCM(self, max_force=1):
                 ].T
                 continue
 
-            temp_tensor = Tensor(np.random.rand(3, 3) - 0.5) * max_force
+            temp_tensor = Tensor(np.random.default_rng().random((3, 3)) - 0.5) * max_force
 
             temp_tensor_sum = sum(temp_tensor.transform(symm_op) for symm_op in self.sharedops[op[0]][op[1]])
             temp_tensor_sum = temp_tensor_sum / (len(self.sharedops[op[0]][op[1]]))
@@ -484,9 +484,10 @@ def get_stable_FCM(self, fcm, fcmasum=10):
 
             max_eig = np.max(-1 * eigs)
             eig_sort = np.argsort(np.abs(eigs))
+            rng = np.random.default_rng()
             for idx in range(3, len(eigs)):
                 if eigs[eig_sort[idx]] > 1e-6:
-                    eigs[eig_sort[idx]] = -1 * max_eig * np.random.rand()
+                    eigs[eig_sort[idx]] = -1 * max_eig * rng.random()
             diag = np.real(np.eye(len(fcm)) * eigs)
 
             fcm = np.real(np.matmul(np.matmul(vecs, diag), vecs.T))

src/pymatgen/analysis/surface_analysis.py

@@ -36,7 +36,6 @@
 
 import copy
 import itertools
-import random
 import warnings
 from typing import TYPE_CHECKING
 
@@ -801,14 +800,15 @@ def color_palette_dict(self, alpha=0.35):
             surface will be transparent.
         """
         color_dict = {}
+        rng = np.random.default_rng()
         for hkl in self.all_slab_entries:
             rgb_indices = [0, 1, 2]
             color = [0, 0, 0, 1]
-            random.shuffle(rgb_indices)
+            rng.shuffle(rgb_indices)
             for idx, ind in enumerate(rgb_indices):
                 if idx == 2:
                     break
-                color[ind] = np.random.uniform(0, 1)
+                color[ind] = rng.uniform(0, 1)
 
             # Get the clean (solid) colors first
             clean_list = np.linspace(0, 1, len(self.all_slab_entries[hkl]))

src/pymatgen/cli/pmg_config.py

@@ -143,7 +143,7 @@ def setup_potcars(potcar_dirs: list[str]):
                     dest = os.path.join(base_dir, os.path.basename(fname))
                     shutil.copy(fname, dest)
                     ext = fname.split(".")[-1]
-                    if ext.upper() in ["Z", "GZ"]:
+                    if ext.upper() in {"Z", "GZ"}:
                         with subprocess.Popen(["gunzip", dest]) as process:
                             process.communicate()
                     elif ext.upper() == "BZ2":
@@ -202,8 +202,8 @@ def build_bader(fortran_command="gfortran"):
     cwd = os.getcwd()
     state = True
     try:
-        urlretrieve(bader_url, "bader.tar.gz")
-        subprocess.call(["tar", "-zxf", "bader.tar.gz"])
+        urlretrieve(bader_url, "bader.tar.gz")  # noqa: S310
+        subprocess.call(["/usr/bin/tar", "-zxf", "bader.tar.gz"])
         os.chdir("bader")
         subprocess.call(["cp", "makefile.osx_" + fortran_command, "makefile"])
         subprocess.call(["make"])

src/pymatgen/command_line/mcsqs_caller.py

@@ -178,7 +178,11 @@ def _parse_sqs_path(path) -> Sqs:
     detected_instances = len(list(path.glob("bestsqs*[0-9]*.out")))
 
     # Convert best SQS structure to CIF file and pymatgen Structure
-    with Popen("str2cif < bestsqs.out > bestsqs.cif", shell=True, cwd=path) as process:
+    with (
+        open(os.path.join(path, "bestsqs.out")) as input_file,
+        open(os.path.join(path, "bestsqs.cif"), "w") as output_file,
+    ):
+        process = Popen(["str2cif"], stdin=input_file, stdout=output_file, cwd=path)
         process.communicate()
 
     with warnings.catch_warnings():
@@ -197,12 +201,15 @@ def _parse_sqs_path(path) -> Sqs:
     all_sqs = []
 
     for idx in range(detected_instances):
-        sqs_out = f"bestsqs{idx + 1}.out"
-        sqs_cif = f"bestsqs{idx + 1}.cif"
-        corr_out = f"bestcorr{idx + 1}.out"
-        with Popen(f"str2cif < {sqs_out} > {sqs_cif}", shell=True, cwd=path) as process:
+        sqs_out = os.path.join(path, f"bestsqs{idx + 1}.out")
+        sqs_cif = os.path.join(path, f"bestsqs{idx + 1}.cif")
+
+        with open(sqs_out) as input_file, open(sqs_cif, "w") as output_file:
+            process = Popen(["str2cif"], stdin=input_file, stdout=output_file, cwd=path)
             process.communicate()
         sqs = Structure.from_file(path / sqs_out)
+
+        corr_out = f"bestcorr{idx + 1}.out"
         with open(path / corr_out) as file:
             lines = file.readlines()
 

src/pymatgen/core/sites.py

@@ -90,8 +90,8 @@ def __getitem__(self, el: Element) -> float:  # type: ignore[override]
 
     def __eq__(self, other: object) -> bool:
         """Site is equal to another site if the species and occupancies are the
-        same, and the coordinates are the same to some tolerance.  numpy
-        function `allclose` is used to determine if coordinates are close.
+        same, and the coordinates are the same to some tolerance. `numpy.allclose`
+        is used to determine if coordinates are close.
         """
         if not isinstance(other, type(self)):
             return NotImplemented