Install some optional dependencies in CI (#3786)

* enable `boltztrap2` * Revert "enable `boltztrap2`" This reverts commit 3b1b077. * use explicit encoding * install graphviz * increment boltztrap version to 24.1.1 * fix bader patch * pre-commit auto-fixes * ruff fix * tweak bader test * need confirm: add `wheel` to dev dep * try install BoltzTraP2 with pip * add comment * try move bolt after numpy to fix win, remove wheel * try move boltrap2 to ubuntu only * more description more accurate * install BoltzTraP2 in CI * try to install BoltzTraP2 with uv * remove repeated numpy install in CI tests * update np.exceptions.RankWarning * Revert "update np.exceptions.RankWarning" This reverts commit 4128284. * install setuptool with uv * update BoltzTraP2 * install wheel for building boltztrap2 * try to install boltztrap2 with pip not uv * install cython before boltztrap2 * try to install boltztrap2 with optional * try to manually install bt2 in workflow * manually install bt2 with pip * remove bt2 ver * install bt2 only on ubuntu * skip bt2 for windows * try to fix failing bt2 test * more descriptive skip msg * finish typing of bzt2 * try to install pygraphviz * revert update to | replace for graphs * test.yml join install pymatgen and install dependencies steps * fix TestStructureGraph.test_draw unhashable type list error * skip BoltzTraP2 and matplotlib 3.9.1 --------- Co-authored-by: Janosh Riebesell <[email protected]>
materialsproject · Aug 6, 2024 · 7a01f3c · 7a01f3c
1 parent 99f62d2
commit 7a01f3c
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 46 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -35,11 +35,11 @@ jobs:
             resolution: highest
             extras: ci,optional
           - os: ubuntu-latest
-            python: '>3.9'
+            python: ">3.9"
             resolution: lowest-direct
             extras: ci,optional
           - os: macos-latest
-            python: '3.10'
+            python: "3.10"
             resolution: lowest-direct
             extras: ci # test with only required dependencies installed
 
@@ -70,18 +70,29 @@ jobs:
       - name: Install ubuntu-only conda dependencies
         if: matrix.config.os == 'ubuntu-latest'
         run: |
-          micromamba install -n pmg -c conda-forge enumlib packmol bader openbabel openff-toolkit --yes
+          micromamba install -n pmg -c conda-forge enumlib packmol bader openbabel openff-toolkit pygraphviz --yes
 
       - name: Install pymatgen and dependencies
         run: |
           micromamba activate pmg
+
           # TODO remove temporary fix. added since uv install torch is flaky.
           # track https://github.com/astral-sh/uv/issues/1921 for resolution
           pip install torch --upgrade
 
-          uv pip install numpy cython
+          uv pip install cython setuptools wheel
+
           uv pip install --editable '.[${{ matrix.config.extras }}]' --resolution=${{ matrix.config.resolution }}
 
+      - name: Install optional Ubuntu dependencies
+        if: matrix.config.os == 'ubuntu-latest'
+        run: |
+          micromamba activate pmg
+
+          # TODO: uv cannot install BoltzTraP2 (#3786),
+          # suggesting no NumPy when there is
+          pip install BoltzTraP2
+
       - name: pytest split ${{ matrix.split }}
         run: |
           micromamba activate pmg

diff --git a/pyproject.toml b/pyproject.toml
@@ -95,10 +95,8 @@ ci = ["pytest-cov>=4", "pytest-split>=0.8", "pytest>=8"]
 docs = ["sphinx", "sphinx_rtd_theme"]
 optional = [
     "ase>=3.23.0",
-    # TODO restore BoltzTraP2 when install fixed, hopefully following merge of
-    # https://gitlab.com/sousaw/BoltzTraP2/-/merge_requests/18
-    # caused CI failure due to ModuleNotFoundError: No module named 'packaging'
-    # "BoltzTraP2>=22.3.2; platform_system!='Windows'",
+    # TODO: uv cannot install BoltzTraP2
+    # "BoltzTraP2>=24.7.2 ; platform_system != 'Windows'",
     "chemview>=0.6",
     "chgnet>=0.3.8",
     "f90nml>=1.1.2",
@@ -107,7 +105,7 @@ optional = [
     "jarvis-tools>=2020.7.14",
     "matgl>=1.1.1",
     # TODO: track https://github.com/matplotlib/matplotlib/issues/28551
-    "matplotlib>=3.8,<3.9.1",
+    "matplotlib>=3.8,!=3.9.1",
     "netCDF4>=1.6.5",
     "phonopy>=2.23",
     "seekpath>=2.0.1",

diff --git a/src/pymatgen/analysis/graphs.py b/src/pymatgen/analysis/graphs.py
@@ -933,7 +933,7 @@ def draw_graph_to_file(
                     d["label"] = f"{d['weight']:.2f} {units}"
 
             # update edge with our new style attributes
-            g.edges[u, v, k] |= d
+            g.edges[u, v, k].update(d)
 
         # optionally remove periodic image edges,
         # these can be confusing due to periodic boundaries
@@ -2603,7 +2603,7 @@ def draw_graph_to_file(
                     d["label"] = f"{d['weight']:.2f} {units}"
 
             # update edge with our new style attributes
-            g.edges[u, v, k] |= d
+            g.edges[u, v, k].update(d)
 
         # optionally remove periodic image edges,
         # these can be confusing due to periodic boundaries

diff --git a/src/pymatgen/electronic_structure/boltztrap2.py b/src/pymatgen/electronic_structure/boltztrap2.py
@@ -44,6 +44,7 @@
 
 if TYPE_CHECKING:
     from pathlib import Path
+    from typing import Literal
 
     from typing_extensions import Self
 
@@ -961,20 +962,20 @@ def __init__(self, bzt_transP=None, bzt_interp=None) -> None:
 
     def plot_props(
         self,
-        prop_y,
-        prop_x,
-        prop_z="temp",
-        output="avg_eigs",
-        dop_type="n",
-        doping=None,
-        temps=None,
-        xlim=(-2, 2),
-        ax: plt.Axes = None,
-    ):
+        prop_y: str,
+        prop_x: Literal["mu", "doping", "temp"],
+        prop_z: Literal["doping", "temp"] = "temp",
+        output: Literal["avg_eigs", "eigs"] = "avg_eigs",
+        dop_type: Literal["n", "p"] = "n",
+        doping: list[float] | None = None,
+        temps: list[float] | None = None,
+        xlim: tuple[float, float] = (-2, 2),
+        ax: plt.Axes | None = None,
+    ) -> plt.Axes | plt.Figure:
         """Plot the transport properties.
 
         Args:
-            prop_y: property to plot among ("Conductivity","Seebeck","Kappa","Carrier_conc",
+            prop_y: property to plot among ("Conductivity", "Seebeck", "Kappa", "Carrier_conc",
                 "Hall_carrier_conc_trace"). Abbreviations are possible, like "S" for "Seebeck"
             prop_x: independent variable in the x-axis among ('mu','doping','temp')
             prop_z: third variable to plot multiple curves ('doping','temp')
@@ -991,7 +992,9 @@ def plot_props(
             ax: figure.axes where to plot. If None, a new figure is produced.
 
         Returns:
-            plt.Axes: matplotlib Axes object
+            plt.Axes: matplotlib Axes object if ax provided
+            OR
+            plt.Figure: matplotlib Figure object if ax is None
 
         Example:
             bztPlotter.plot_props('S','mu','temp',temps=[600,900,1200]).show()
@@ -1026,15 +1029,15 @@ def plot_props(
             r"$(cm^{-3})$",
         )
 
-        props_short = [p[: len(prop_y)] for p in props]
+        props_short = tuple(p[: len(prop_y)] for p in props)
 
         if prop_y not in props_short:
             raise BoltztrapError("prop_y not valid")
 
-        if prop_x not in ("mu", "doping", "temp"):
+        if prop_x not in {"mu", "doping", "temp"}:
             raise BoltztrapError("prop_x not valid")
 
-        if prop_z not in ("doping", "temp"):
+        if prop_z not in {"doping", "temp"}:
             raise BoltztrapError("prop_z not valid")
 
         idx_prop = props_short.index(prop_y)
@@ -1048,8 +1051,7 @@ def plot_props(
         else:
             p_array = getattr(self.bzt_transP, f"{props[idx_prop]}_{prop_x}")
 
-        if ax is None:
-            plt.figure(figsize=(10, 8))
+        fig = plt.figure(figsize=(10, 8)) if ax is None else None
 
         temps_all = self.bzt_transP.temp_r.tolist()
         if temps is None:
@@ -1112,6 +1114,9 @@ def plot_props(
             leg_title = f"{dop_type}-type"
 
         elif prop_z == "doping" and prop_x == "temp":
+            if doping is None:
+                raise ValueError("doping cannot be None when prop_z is doping")
+
             for dop in doping:
                 dop_idx = doping_all.index(dop)
                 prop_out = np.linalg.eigh(p_array[dop_type][:, dop_idx])[0]
@@ -1137,10 +1142,11 @@ def plot_props(
         plt.ylabel(f"{props_lbl[idx_prop]} {props_unit[idx_prop]}", fontsize=30)
         plt.xticks(fontsize=25)
         plt.yticks(fontsize=25)
-        plt.legend(title=leg_title if leg_title != "" else "", fontsize=15)
+        plt.legend(title=leg_title or "", fontsize=15)
         plt.tight_layout()
         plt.grid()
-        return ax
+
+        return fig if ax is None else ax
 
     def plot_bands(self):
         """Plot a band structure on symmetry line using BSPlotter()."""

diff --git a/tests/command_line/test_bader_caller.py b/tests/command_line/test_bader_caller.py
@@ -2,7 +2,6 @@
 
 import warnings
 from shutil import which
-from unittest.mock import patch
 
 import numpy as np
 import pytest
@@ -60,7 +59,6 @@ def test_init(self):
         assert len(analysis.data) == 14
 
         # Test Cube file format parsing
-
         copy_r(TEST_DIR, self.tmp_path)
         analysis = BaderAnalysis(cube_filename=f"{TEST_DIR}/elec.cube.gz")
         assert len(analysis.data) == 9
@@ -76,17 +74,17 @@ def test_from_path(self):
         analysis = BaderAnalysis(chgcar_filename=chgcar_path, chgref_filename=chgref_path)
         analysis_from_path = BaderAnalysis.from_path(from_path_dir)
 
-        for key in analysis_from_path.summary:
-            val, val_from_path = analysis.summary[key], analysis_from_path.summary[key]
-            if isinstance(analysis_from_path.summary[key], (bool, str)):
+        for key, val_from_path in analysis_from_path.summary.items():
+            val = analysis.summary[key]
+            if isinstance(val_from_path, (bool, str)):
                 assert val == val_from_path, f"{key=}"
             elif key == "charge":
                 assert_allclose(val, val_from_path, atol=1e-5)
 
     def test_bader_analysis_from_path(self):
-        summary = bader_analysis_from_path(TEST_DIR)
         """
         Reference summary dict (with bader 1.0)
+
         summary_ref = {
             "magmom": [4.298761, 4.221997, 4.221997, 3.816685, 4.221997, 4.298763, 0.36292, 0.370516, 0.36292,
                 0.36292, 0.36292, 0.36292, 0.36292, 0.370516],
@@ -102,6 +100,9 @@ def test_bader_analysis_from_path(self):
             "reference_used": True,
         }
         """
+
+        summary = bader_analysis_from_path(TEST_DIR)
+
         assert set(summary) == {
             "magmom",
             "min_dist",
@@ -131,12 +132,11 @@ def test_atom_parsing(self):
         )
 
     def test_missing_file_bader_exe_path(self):
-        pytest.skip("doesn't reliably raise RuntimeError")
-        # mock which("bader") to return None so we always fall back to use bader_exe_path
-        with (
-            patch("shutil.which", return_value=None),
-            pytest.raises(
-                RuntimeError, match="BaderAnalysis requires the executable bader be in the PATH or the full path "
-            ),
-        ):
-            BaderAnalysis(chgcar_filename=f"{VASP_OUT_DIR}/CHGCAR.Fe3O4.gz", bader_exe_path="")
+        # Mock which("bader") to return None so we always fall back to use bader_exe_path
+        with pytest.MonkeyPatch.context() as monkeypatch:
+            monkeypatch.setenv("PATH", "")
+
+            with pytest.raises(
+                RuntimeError, match="Requires bader or bader.exe to be in the PATH or the absolute path"
+            ):
+                BaderAnalysis(chgcar_filename=f"{VASP_OUT_DIR}/CHGCAR.Fe3O4.gz")
diff --git a/tests/electronic_structure/test_boltztrap2.py b/tests/electronic_structure/test_boltztrap2.py
@@ -316,7 +316,9 @@ def test_plot(self):
         assert self.bztPlotter is not None
         fig = self.bztPlotter.plot_props("S", "mu", "temp", temps=[300, 500])
         assert fig is not None
+
         fig = self.bztPlotter.plot_bands()
         assert fig is not None
+
         fig = self.bztPlotter.plot_dos()
         assert fig is not None
diff --git a/tests/ext/test_cod.py b/tests/ext/test_cod.py
@@ -11,7 +11,7 @@
 
 if "CI" in os.environ:  # test is slow and flaky, skip in CI. see
     # https://github.com/materialsproject/pymatgen/pull/3777#issuecomment-2071217785
-    pytest.skip(allow_module_level=True)
+    pytest.skip(allow_module_level=True, reason="Skip COD test in CI")
 
 try:
     website_down = requests.get("https://www.crystallography.net", timeout=600).status_code != 200