Fix release CI (#161)

* pin cython>=3

* update pypa/cibuildwheel to v2.18.1

* bump pre-commit hooks and unignore ruff PT011 PT013

* ruff unignore PLR, only ignore specific PLR0912 PLR0913 PLR0915

* try removing name and path from actions/download-artifact@v4 to (hopefully) download ALL artifacts

.github/workflows/release.yml

@@ -51,7 +51,7 @@ jobs:
         uses: actions/checkout@v4
 
       - name: Build wheels
-        uses: pypa/cibuildwheel@v2.16.5
+        uses: pypa/cibuildwheel@v2.18.1
         env:
           CIBW_BUILD: cp${{ matrix.python-version }}-*
           CIBW_ARCHS_MACOS: universal2
@@ -74,10 +74,8 @@ jobs:
       - name: Download build artifacts
         uses: actions/download-artifact@v4
         with:
-          name: artifact
           path: dist
           merge-multiple: true
-          pattern: dist-*
 
       - name: Publish to PyPi or TestPyPI
         uses: pypa/gh-action-pypi-publish@release/v1

.pre-commit-config.yaml

@@ -4,7 +4,7 @@ default_install_hook_types: [pre-commit, commit-msg]
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.4.4
+    rev: v0.4.7
     hooks:
       - id: ruff
         args: [--fix]
@@ -23,7 +23,7 @@ repos:
       - id: trailing-whitespace
 
   - repo: https://github.com/codespell-project/codespell
-    rev: v2.2.6
+    rev: v2.3.0
     hooks:
       - id: codespell
         stages: [commit, commit-msg]
@@ -46,7 +46,7 @@ repos:
           - svelte
 
   - repo: https://github.com/pre-commit/mirrors-eslint
-    rev: v9.3.0
+    rev: v9.4.0
     hooks:
       - id: eslint
         types: [file]

chgnet/graph/graph.py

@@ -127,14 +127,18 @@ def __init__(self, nodes: list[Node]) -> None:
         self.undirected_edges: dict[frozenset[int], list[UndirectedEdge]] = {}
         self.undirected_edges_list: list[UndirectedEdge] = []
 
-    def add_edge(self, center_index, neighbor_index, image, distance) -> None:
+    def add_edge(
+        self, center_index, neighbor_index, image, distance, dist_tol: float = 1e-6
+    ) -> None:
         """Add an directed edge to the graph.
 
         Args:
             center_index (int): center node index
             neighbor_index (int): neighbor node index
             image (np.array): the periodic cell image the neighbor is from
             distance (float): distance between center and neighbor.
+            dist_tol (float): tolerance for distance comparison between edges.
+                Default = 1e-6
         """
         # Create directed_edge (DE) index using the length of added DEs
         directed_edge_index = len(self.directed_edges_list)
@@ -173,7 +177,7 @@ def add_edge(self, center_index, neighbor_index, image, distance) -> None:
             # different image and distance (this is possible consider periodicity)
             for undirected_edge in self.undirected_edges[tmp]:
                 if (
-                    abs(undirected_edge.info["distance"] - distance) < 1e-6
+                    abs(undirected_edge.info["distance"] - distance) < dist_tol
                     and len(undirected_edge.info["directed_edge_index"]) == 1
                 ):
                     # There is an undirected edge with similar length and only one of
@@ -286,7 +290,7 @@ def line_graph_adjacency_list(self, cutoff) -> tuple[list[list[int]], list[int]]
             # if encountered exception,
             # it means after Atom_Graph creation, the UDE has only 1 DE associated
             # This exception is not encountered from the develop team's experience
-            if len(u_edge.info["directed_edge_index"]) != 2:
+            if len(u_edge.info["directed_edge_index"]) != 2:  # noqa: PLR2004
                 raise ValueError(
                     "Did not find 2 Directed_edges !!!"
                     f"undirected edge {u_edge} has:"

chgnet/model/dynamics.py

@@ -262,7 +262,7 @@ def relax(
             dict[str, Structure | TrajectoryObserver]:
                 A dictionary with 'final_structure' and 'trajectory'.
         """
-        import ase.filters as filters
+        from ase import filters
         from ase.filters import Filter
 
         valid_filter_names = [

pyproject.toml

@@ -8,7 +8,7 @@ readme = "README.md"
 license = { text = "Modified BSD" }
 dependencies = [
     "ase>=3.23.0",
-    "cython>=0.29.26",
+    "cython>=3",
     "numpy>=1.26",
     "nvidia-ml-py3>=7.352.0",
     "pymatgen>=2023.10.11",
@@ -73,12 +73,11 @@ ignore = [
     "ERA001",  # found commented out code
     "ISC001",
     "NPY002",  # TODO replace legacy np.random.seed
-    "PLR",     # pylint refactor
+    "PLR0912", # too many branches
+    "PLR0913", # too many args in function def
+    "PLR0915", # too many statements
     "PLW2901", # Outer for loop variable overwritten by inner assignment target
     "PT006",   # pytest-parametrize-names-wrong-type
-    "PT011",   # pytest-raises-too-broad
-    "PT013",   # pytest-incorrect-pytest-import
-    "PT019",   # pytest-fixture-param-without-value
     "PTH",     # prefer Path to os.path
     "S108",
     "S301",    # pickle can be unsafe
@@ -96,7 +95,16 @@ docstring-code-format = true
 
 [tool.ruff.lint.per-file-ignores]
 "site/*" = ["INP001", "S602"]
-"tests/*" = ["ANN201", "D100", "D103", "FBT001", "FBT002", "INP001", "S101"]
+"tests/*" = [
+    "ANN201",
+    "D100",
+    "D103",
+    "FBT001",
+    "FBT002",
+    "INP001",
+    "PLR2004",
+    "S101",
+]
 # E402 Module level import not at top of file
 "examples/*" = ["E402", "I002", "INP001", "N816", "S101", "T201"]
 "chgnet/**/*" = ["T201"]

site/make_docs.py

@@ -33,7 +33,8 @@
 
     # remove all files with less than 20 lines
     # these correspond to mostly empty __init__.py files
-    if markdown.count("\n") < 20:
+    min_line_cnt = 20
+    if markdown.count("\n") < min_line_cnt:
         os.remove(path)
         continue
 

tests/test_converter.py

@@ -1,22 +1,24 @@
 from __future__ import annotations
 
-from typing import Literal
+from typing import TYPE_CHECKING, Literal
 
 import pytest
 from pymatgen.core import Lattice, Structure
-from pytest import CaptureFixture
 
 from chgnet.graph import CrystalGraph
 from chgnet.graph.converter import CrystalGraphConverter
 
+if TYPE_CHECKING:
+    from collections.abc import Generator
+
 lattice = Lattice.cubic(4)
 species = ["Na", "Cl"]
 coords = [[0, 0, 0], [0.5, 0.5, 0.5]]
 NaCl = Structure(lattice, species, coords)
 
 
 @pytest.fixture()
-def _set_make_graph() -> None:
+def _set_make_graph() -> Generator[None, None, None]:
     # fixture to force make_graph to be None and then restore it after test
     from chgnet.graph import converter
 
@@ -63,7 +65,7 @@ def test_crystal_graph_converter_warns():
 
 @pytest.mark.parametrize("on_isolated_atoms", ["ignore", "warn", "error"])
 def test_crystal_graph_converter_forward(
-    on_isolated_atoms, capsys: CaptureFixture[str]
+    on_isolated_atoms, capsys: pytest.CaptureFixture[str]
 ):
     atom_graph_cutoff = 5
     converter = CrystalGraphConverter(
@@ -75,13 +77,15 @@ def test_crystal_graph_converter_forward(
     strained.apply_strain(5)
     graph_id = "strained"
     err_msg = (
-        f"Structure {graph_id=} has 2 isolated atom(s) with "
+        f"Structure {graph_id=} has {len(NaCl)} isolated atom(s) with "
         f"{atom_graph_cutoff=}. "
         f"CHGNet calculation will likely go wrong"
     )
 
     if on_isolated_atoms == "error":
-        with pytest.raises(ValueError) as exc_info:
+        with pytest.raises(
+            ValueError, match=f"Structure {graph_id=} has {len(NaCl)} isolated atom"
+        ) as exc_info:
             converter.forward(strained, graph_id=graph_id)
         assert err_msg in str(exc_info.value)
     else:

tests/test_encoders.py

@@ -58,11 +58,9 @@ def test_angle_encoder(num_angular: int, learnable: bool) -> None:
 
 @pytest.mark.parametrize("num_angular", [-2, 8])
 def test_angle_encoder_num_angular(num_angular: int) -> None:
-    with pytest.raises(ValueError) as exc_info:
+    with pytest.raises(ValueError, match=f"{num_angular=} must be an odd integer"):
         AngleEncoder(num_angular=num_angular)
 
-    assert f"{num_angular=} must be an odd integer" in str(exc_info.value)
-
 
 @pytest.mark.parametrize("learnable", [True, False])
 def test_bond_encoder_learnable(learnable: bool) -> None:

tests/test_graph.py

@@ -100,7 +100,7 @@ def test_directed_edge() -> None:
     info = {"image": np.zeros(3), "distance": 1}
     edge = DirectedEdge([0, 1], index=0, info=info)
     undirected = edge.make_undirected(index=0, info=info)
-    assert edge == edge
+    assert edge == edge  # noqa: PLR0124
     assert edge == undirected
     assert edge.nodes == [0, 1]
     assert edge.index == 0

tests/test_md.py

@@ -15,7 +15,7 @@
 from pymatgen.analysis.structure_matcher import StructureMatcher
 from pymatgen.core import Structure
 from pymatgen.io.ase import AseAtomsAdaptor
-from pytest import MonkeyPatch, approx
+from pytest import MonkeyPatch, approx  # noqa: PT013
 
 from chgnet import ROOT
 from chgnet.graph import CrystalGraphConverter

tests/test_model.py

@@ -3,7 +3,6 @@
 import numpy as np
 import pytest
 from pymatgen.core import Structure
-from pytest import mark
 
 from chgnet import ROOT
 from chgnet.graph import CrystalGraphConverter
@@ -14,13 +13,13 @@
 model = CHGNet.load()
 
 
-@mark.parametrize("atom_fea_dim", [1, 64])
-@mark.parametrize("bond_fea_dim", [1, 64])
-@mark.parametrize("angle_fea_dim", [1, 64])
-@mark.parametrize("num_radial", [1, 9])
-@mark.parametrize("num_angular", [1, 9])
-@mark.parametrize("n_conv", [1, 4])
-@mark.parametrize("composition_model", ["MPtrj", "MPtrj_e", "MPF"])
+@pytest.mark.parametrize("atom_fea_dim", [1, 64])
+@pytest.mark.parametrize("bond_fea_dim", [1, 64])
+@pytest.mark.parametrize("angle_fea_dim", [1, 64])
+@pytest.mark.parametrize("num_radial", [1, 9])
+@pytest.mark.parametrize("num_angular", [1, 9])
+@pytest.mark.parametrize("n_conv", [1, 4])
+@pytest.mark.parametrize("composition_model", ["MPtrj", "MPtrj_e", "MPF"])
 def test_model(
     atom_fea_dim: int,
     bond_fea_dim: int,
@@ -118,8 +117,8 @@ def test_predict_structure() -> None:
     assert out["atom_fea"].shape == (8, 64)
 
 
-@mark.parametrize("axis", [[0, 0, 1], [1, 1, 0], [-2, 3, 1]])
-@mark.parametrize("rotation_angle", [5, 30, 45, 120])
+@pytest.mark.parametrize("axis", [[0, 0, 1], [1, 1, 0], [-2, 3, 1]])
+@pytest.mark.parametrize("rotation_angle", [5, 30, 45, 120])
 def test_predict_structure_rotated(rotation_angle: float, axis: list) -> None:
     from pymatgen.transformations.standard_transformations import RotationTransformation
 

tests/test_relaxation.py

@@ -8,7 +8,6 @@
 import torch
 from ase.filters import ExpCellFilter, Filter, FrechetCellFilter
 from pymatgen.core import Structure
-from pytest import approx, mark, param
 
 from chgnet.graph import CrystalGraphConverter
 from chgnet.model import CHGNet, StructOptimizer
@@ -54,19 +53,20 @@ def test_relaxation(
     assert len(traj) == 2 if algorithm == "legacy" else 4
 
     # make sure final structure is more relaxed than initial one
-    assert traj.energies[-1] == approx(-58.94209, rel=1e-4)
+    assert traj.energies[-1] == pytest.approx(-58.94209, rel=1e-4)
 
 
-no_cuda = mark.skipif(not torch.cuda.is_available(), reason="No CUDA device")
+no_cuda = pytest.mark.skipif(not torch.cuda.is_available(), reason="No CUDA device")
 # skip in macos-14 M1 CI due to OOM error (TODO investigate if
 # PYTORCH_MPS_HIGH_WATERMARK_RATIO can fix)
-no_mps = mark.skipif(
+no_mps = pytest.mark.skipif(
     not torch.backends.mps.is_available() or "CI" in os.environ, reason="No MPS device"
 )
 
 
-@mark.parametrize(
-    "use_device", ["cpu", param("cuda", marks=no_cuda), param("mps", marks=no_mps)]
+@pytest.mark.parametrize(
+    "use_device",
+    ["cpu", pytest.param("cuda", marks=no_cuda), pytest.param("mps", marks=no_mps)],
 )
 def test_structure_optimizer_passes_kwargs_to_model(use_device: str) -> None:
     relaxer = StructOptimizer(use_device=use_device)