Merge pull request #760 from janosh/main

Fix `ModuleNotFoundError`: No module named 'pymatgen.transformations.defect_transformations'

.github/workflows/test.yml

@@ -18,11 +18,15 @@ jobs:
 
     steps:
       - name: Checkout repo
-        uses: actions/checkout@v2
+        uses: actions/checkout@v3
 
       - name: Setup Python
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v4
         with:
+          cache: pip
+          cache-dependency-path: |
+            setup.py
+            requirements-ci.txt
           python-version: 3.8
 
       - name: Install OpenBabel
@@ -32,14 +36,6 @@ jobs:
           # https://github.com/openbabel/openbabel/issues/2408#issuecomment-1014466193
           sudo ln -s /usr/include/openbabel3 /usr/local/include/openbabel3
 
-      - name: Cache pip
-        uses: actions/cache@v2
-        with:
-          path: ~/.cache/pip
-          key: ${{ runner.os }}-pip-${{ hashFiles('requirements-ci.txt', 'setup.py') }}
-          restore-keys: |
-            ${{ runner.os }}-pip-
-
       - name: Install dependencies
         run: |
           pip install -r requirements-ci.txt

.pre-commit-config.yaml

@@ -1,36 +1,64 @@
+ci:
+  autoupdate_schedule: quarterly
+  skip: [mypy]
+
+default_stages: [commit]
+
+default_install_hook_types: [pre-commit, commit-msg]
+
 exclude: ^(docs/|.*test_files/|.*tests/)
 
 repos:
   - repo: https://github.com/myint/autoflake
     rev: v1.4
     hooks:
       - id: autoflake
-        args: [--in-place, --remove-all-unused-imports, --remove-unused-variable, --ignore-init-module-imports]
+        args:
+          - --in-place
+          - --remove-unused-variables
+          - --remove-all-unused-imports
+          - --expand-star-imports
+          - --ignore-init-module-imports
 
   - repo: https://github.com/psf/black
-    rev: 22.1.0
+    rev: 22.6.0
     hooks:
       - id: black
 
-  - repo: https://github.com/pycqa/flake8
-    rev: 4.0.1
+  - repo: https://github.com/PyCQA/flake8
+    rev: 5.0.4
     hooks:
       - id: flake8
-        args: [--max-line-length=125]
+
+  - repo: https://github.com/pre-commit/mirrors-mypy
+    rev: v0.971
+    hooks:
+      - id: mypy
+        additional_dependencies: [types-requests]
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.1.0
+    rev: v4.3.0
     hooks:
+      - id: check-case-conflict
+      - id: check-symlinks
       - id: check-yaml
+      - id: destroyed-symlinks
       - id: end-of-file-fixer
+      - id: mixed-line-ending
       - id: trailing-whitespace
 
   - repo: https://github.com/asottile/pyupgrade
-    rev: v2.31.0
+    rev: v2.37.3
     hooks:
       - id: pyupgrade
         args: [--py38-plus]
 
+  - repo: https://github.com/codespell-project/codespell
+    rev: v2.2.1
+    hooks:
+      - id: codespell
+        stages: [commit, commit-msg]
+
   - repo: https://github.com/PyCQA/isort
     rev: 5.10.1
     hooks:

atomate/feff/workflows/tests/test_eels_workflows.py

@@ -14,9 +14,8 @@
 
 module_dir = os.path.dirname(os.path.abspath(__file__))
 db_dir = os.path.join(module_dir, "..", "..", "..", "common", "test_files")
-DEBUG_MODE = (
-    False  # If true, retains the database and output dirs at the end of the test
-)
+# If DEBUG_MODE = true, retains the database and output dirs at the end of the test
+DEBUG_MODE = False
 
 
 class TestEELSWorkflow(AtomateTest):

atomate/feff/workflows/tests/test_exafs_scattering_paths.py

@@ -41,7 +41,6 @@ def test_feff_input_sets(self):
             "nkpts": 1000,
             "radius": 10.0,
             "user_tag_settings": {},
-            "structure": self.struct.as_dict(),
         }
         ans_fis_fw2 = {
             "@class": "MPEXAFSSet",
@@ -51,13 +50,14 @@ def test_feff_input_sets(self):
             "nkpts": 1000,
             "radius": 10.0,
             "user_tag_settings": {"CONTROL": "0 0 0 0 1 1", "PRINT": "0 0 0 1 0 3"},
-            "structure": self.struct.as_dict(),
         }
         fis_fw1 = self.fw1_dict["spec"]["_tasks"][0]["feff_input_set"]
         fis_fw2 = self.fw2_dict["spec"]["_tasks"][1]["feff_input_set"]
 
         fis_fw1.pop("@version")
         fis_fw2.pop("@version")
+        fis_fw1.pop("structure")
+        fis_fw2.pop("structure")
 
         self.assertDictEqual(fis_fw1, ans_fis_fw1)
         self.assertDictEqual(fis_fw2, ans_fis_fw2)

atomate/feff/workflows/tests/test_xas_workflows.py

@@ -16,9 +16,8 @@
 module_dir = os.path.dirname(os.path.abspath(__file__))
 db_dir = os.path.join(module_dir, "..", "..", "..", "common", "test_files")
 
-DEBUG_MODE = (
-    False  # If true, retains the database and output dirs at the end of the test
-)
+# If DEBUG_MODE = true, retains the database and output dirs at the end of the test
+DEBUG_MODE = False
 FEFF_CMD = None  # "feff"
 
 

atomate/utils/testing.py

@@ -14,12 +14,10 @@
 MODULE_DIR = os.path.dirname(os.path.abspath(__file__))
 DB_DIR = os.path.join(MODULE_DIR, "..", "common", "test_files")
 
-DEBUG_MODE = (
-    False  # If true, retains the database and output dirs at the end of the test
-)
-VASP_CMD = (
-    None  # If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
-)
+# If DEBUG_MODE = true, retains the database and output dirs at the end of the test
+DEBUG_MODE = False
+# If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
+VASP_CMD = None
 
 
 class AtomateTest(unittest.TestCase):

atomate/vasp/analysis/linear_response.py

@@ -253,29 +253,32 @@ def det_deriv(matrix, i, j):
     jacobians = [[] for i in range(m)]
 
     det = np.linalg.det(matrix)
-    for i in range(m):
-        for j in range(n):
-            mji = np.delete(np.delete(matrix, j, 0), i, 1)
-            minor = (-1) ** (i + j) * np.linalg.det(mji)
+    for idx in range(m):
+        for jdx in range(n):
+            mji = np.delete(np.delete(matrix, jdx, 0), idx, 1)
+            minor = (-1) ** (idx + jdx) * np.linalg.det(mji)
 
             j_matrix = np.zeros([m, n])
-            for k in range(m):
-                for l in range(n):
-                    det_p = det_deriv(matrix, k, l)
+            for kdx in range(m):
+                for ldx in range(n):
+                    det_p = det_deriv(matrix, kdx, ldx)
 
-                    if k == j or l == i:
+                    if kdx == jdx or ldx == idx:
                         minor_p = 0.0
                     else:
-                        kk, ll = k - 1 if k > j else k, l - 1 if l > i else l
-                        minor_p = (-1) ** (i + j) * det_deriv(mji, kk, ll)
+                        kk, ll = (
+                            kdx - 1 if kdx > jdx else kdx,
+                            ldx - 1 if ldx > idx else ldx,
+                        )
+                        minor_p = (-1) ** (idx + jdx) * det_deriv(mji, kk, ll)
 
-                    j_matrix[k, l] = (minor_p * det - minor * det_p) / det**2
+                    j_matrix[kdx, ldx] = (minor_p * det - minor * det_p) / det**2
 
-            jacobians[i].append(j_matrix)
+            jacobians[idx].append(j_matrix)
 
             j_vec = np.reshape(j_matrix, [m * n, 1])
             sigma_f = np.sum(np.dot(np.transpose(j_vec), np.dot(matrix_covar, j_vec)))
-            matrixinv_var[i, j] = sigma_f
+            matrixinv_var[idx, jdx] = sigma_f
 
     return matrixinv, matrixinv_var, jacobians
 

atomate/vasp/firetasks/electrode_tasks.py

@@ -1,7 +1,6 @@
 import math
 
 from fireworks import FiretaskBase, Firework, FWAction, Workflow, explicit_serialize
-from pymatgen.analysis.defects.utils import ChargeInsertionAnalyzer
 from pymatgen.analysis.structure_matcher import StructureMatcher
 from pymatgen.core import Structure
 
@@ -12,6 +11,15 @@
 from atomate.vasp.firetasks import pass_vasp_result
 from atomate.vasp.fireworks.core import OptimizeFW, StaticFW
 
+try:
+    from pymatgen.analysis.defects import ChargeInsertionAnalyzer
+except ImportError:
+    print(
+        "Failed to import ChargeInsertionAnalyzer. This is likely due to converting the pymatgen defects module "
+        "to a namespace package. See https://github.com/materialsproject/pymatgen/pull/2582#issuecomment-1198318101 "
+        "for updates."
+    )
+
 __author__ = "Jimmy Shen"
 __email__ = "jmmshn@lbl.gov"
 
@@ -21,7 +29,7 @@
 
 Note:
 The workflow passes data to fw_spec extensively and requires specific fields in the spec to be updated.
-Example, the base_taks_id must be stored and the spec and updated as the workflow runs so you have to set
+Example, the base_task_id must be stored and the spec and updated as the workflow runs so you have to set
 ```
 {
     "store_volumetric_data": vasptodb_kwargs_vol_data[volumetric_data_type],

atomate/vasp/firetasks/tests/test_polarization_to_db.py

@@ -15,9 +15,8 @@
 DEBUG_MODE = (
     True  # If true, retains the database and output dirs at the end of the test
 )
-VASP_CMD = (
-    None  # If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
-)
+# If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
+VASP_CMD = None
 
 
 class TestFerroelectricWorkflow(AtomateTest):

atomate/vasp/firetasks/tests/test_write_vasp.py

@@ -36,6 +36,7 @@ def setUpClass(cls):
 
     def setUp(self):
         super().setUp(lpad=False)
+        self.maxDiff = None
 
     def tearDown(self):
         for x in ["INCAR", "POSCAR", "POTCAR", "KPOINTS", "POTCAR.spec"]:
@@ -50,7 +51,10 @@ def _verify_files(
             self.assertEqual(Incar.from_file("INCAR"), self.ref_incar)
 
             poscar = Poscar.from_file("POSCAR")
-            self.assertEqual(str(poscar), str(self.ref_poscar))
+            self.assertEqual(
+                poscar.get_string(significant_figures=4),
+                self.ref_poscar.get_string(significant_figures=4),
+            )
 
             if potcar_spec:
                 symbols = Path("POTCAR.spec").read_text().split()

atomate/vasp/fireworks/core.py

@@ -2,7 +2,10 @@
 Defines standardized Fireworks that can be chained easily to perform various
 sequences of VASP calculations.
 """
+from __future__ import annotations
+
 import warnings
+from typing import Any
 
 from fireworks import Firework
 from pymatgen.core import Structure
@@ -583,26 +586,26 @@ def __init__(
 class DFPTFW(Firework):
     def __init__(
         self,
-        structure=None,
-        prev_calc_dir=None,
-        name="static dielectric",
-        vasp_cmd=VASP_CMD,
-        copy_vasp_outputs=True,
-        lepsilon=True,
-        db_file=DB_FILE,
-        parents=None,
-        user_incar_settings=None,
-        pass_nm_results=False,
+        structure: Structure = None,
+        prev_calc_dir: str = None,
+        name: str = "static dielectric",
+        vasp_cmd: str = VASP_CMD,
+        copy_vasp_outputs: bool = True,
+        lepsilon: bool = True,
+        db_file: str = DB_FILE,
+        parents: Firework | list[Firework] = None,
+        user_incar_settings: dict[str, Any] = None,
+        pass_nm_results: bool = False,
         **kwargs,
-    ):
+    ) -> None:
         """
-         Static DFPT calculation Firework
+        Static DFPT calculation Firework
 
         Args:
             structure (Structure): Input structure. If copy_vasp_outputs, used only to set the
                 name of the FW.
             name (str): Name for the Firework.
-            lepsilon (bool): Turn on LEPSILON to calculate polar properties
+            lepsilon (bool): Turn on LEPSILON to calculate polar properties. Defaults to True.
             vasp_cmd (str): Command to run vasp.
             copy_vasp_outputs (str or bool): Whether to copy outputs from previous
                 run. Defaults to True.

atomate/vasp/fireworks/tests/test_fireworks_core.py

@@ -24,12 +24,10 @@
 db_dir = os.path.join(module_dir, "..", "..", "..", "common", "test_files")
 reference_dir = os.path.join(module_dir, "..", "..", "test_files")
 
-DEBUG_MODE = (
-    False  # If true, retains the database and output dirs at the end of the test
-)
-VASP_CMD = (
-    None  # If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
-)
+# If DEBUG_MODE = true, retains the database and output dirs at the end of the test
+DEBUG_MODE = False
+# If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
+VASP_CMD = None
 
 
 class TestCoreFireworks(unittest.TestCase):

atomate/vasp/fireworks/tests/test_nmr.py

@@ -14,12 +14,10 @@
 db_dir = os.path.join(module_dir, "..", "..", "..", "common", "test_files")
 reference_dir = os.path.join(module_dir, "..", "..", "test_files")
 
-DEBUG_MODE = (
-    False  # If true, retains the database and output dirs at the end of the test
-)
-VASP_CMD = (
-    None  # If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
-)
+# If DEBUG_MODE = true, retains the database and output dirs at the end of the test
+DEBUG_MODE = False
+# If None, runs a "fake" VASP. Otherwise, runs VASP with this command...
+VASP_CMD = None
 
 
 class TestNMRFireworks(unittest.TestCase):

atomate/vasp/powerups.py

@@ -214,7 +214,7 @@ def add_trackers(original_wf, tracked_files=None, nlines=25):
     trackers = [Tracker(f, nlines=nlines, allow_zipped=True) for f in tracked_files]
 
     idx_list = get_fws_and_tasks(original_wf, task_name_constraint="RunVasp")
-    for idx_fw, idx_t in idx_list:
+    for idx_fw, _ in idx_list:
         if "_trackers" in original_wf.fws[idx_fw].spec:
             original_wf.fws[idx_fw].spec["_trackers"].extend(trackers)
         else: