Fix bader_analysis_from_path using warning as file path and reinsta…

…te test (#3632) * fix bader_analysis_from_path() misusing _get_filepath() * unskip test_bader_analysis_from_path
materialsproject · Feb 17, 2024 · ff45527 · ff45527
1 parent 5514ff5
commit ff45527
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 22 deletions.
diff --git a/pymatgen/command_line/bader_caller.py b/pymatgen/command_line/bader_caller.py
@@ -435,9 +435,10 @@ def bader_analysis_from_path(path, suffix=""):
         summary dict
     """
 
-    def _get_filepath(filename, path=path, suffix=suffix):
-        paths = glob(f"{path}/{filename}{suffix}*")
+    def _get_filepath(filename: str, msg: str = "") -> str | None:
+        paths = glob(glob_pattern := f"{path}/{filename}{suffix}*")
         if len(paths) == 0:
+            warnings.warn(msg or f"no matches for {glob_pattern=}")
             return None
         if len(paths) > 1:
             # using reverse=True because, if multiple files are present,
@@ -516,17 +517,17 @@ def bader_analysis_from_objects(chgcar, potcar=None, aeccar0=None, aeccar2=None)
             )
 
             summary = {
-                "min_dist": [d["min_dist"] for d in ba.data],
-                "charge": [d["charge"] for d in ba.data],
-                "atomic_volume": [d["atomic_vol"] for d in ba.data],
+                "min_dist": [dct["min_dist"] for dct in ba.data],
+                "charge": [dct["charge"] for dct in ba.data],
+                "atomic_volume": [dct["atomic_vol"] for dct in ba.data],
                 "vacuum_charge": ba.vacuum_charge,
                 "vacuum_volume": ba.vacuum_volume,
                 "reference_used": bool(chgref_path),
                 "bader_version": ba.version,
             }
 
             if potcar:
-                charge_transfer = [ba.get_charge_transfer(i) for i in range(len(ba.data))]
+                charge_transfer = [ba.get_charge_transfer(idx) for idx in range(len(ba.data))]
                 summary["charge_transfer"] = charge_transfer
 
             if chgcar.is_spin_polarized:
@@ -541,7 +542,7 @@ def bader_analysis_from_objects(chgcar, potcar=None, aeccar0=None, aeccar2=None)
                     potcar_filename=potcar_path,
                     chgref_filename=chgref_path,
                 )
-                summary["magmom"] = [d["charge"] for d in ba.data]
+                summary["magmom"] = [dct["charge"] for dct in ba.data]
     finally:
         os.chdir(orig_dir)
 

diff --git a/tests/command_line/test_bader_caller.py b/tests/command_line/test_bader_caller.py
@@ -83,24 +83,23 @@ def test_from_path(self):
             elif key == "charge":
                 assert_allclose(val, val_from_path, atol=1e-5)
 
-    def test_automatic_runner(self):
-        pytest.skip("raises RuntimeError: bader exits with return code 24")
+    def test_bader_analysis_from_path(self):
         summary = bader_analysis_from_path(f"{TEST_FILES_DIR}/bader")
         """
         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],
-            'min_dist': [0.835789, 0.92947, 0.92947, 0.973007, 0.92947, 0.835789, 0.94067,
-                         0.817381, 0.94067, 0.94067, 0.94067, 0.94067, 0.94067, 0.817381],
-            'vacuum_charge': 0.0,
-            'vacuum_volume': 0.0,
-            'atomic_volume': [9.922887, 8.175158, 8.175158, 9.265802, 8.175158, 9.923233, 12.382546,
-                              12.566972, 12.382546, 12.382546, 12.382546, 12.382546, 12.382546, 12.566972],
-            'charge': [12.248132, 12.26177, 12.26177, 12.600596, 12.26177, 12.248143, 7.267303,
-                       7.256998, 7.267303, 7.267303, 7.267303, 7.267303, 7.267303, 7.256998],
-            'bader_version': 1.0,
-            'reference_used': True
+            "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],
+            "min_dist": [0.835789, 0.92947, 0.92947, 0.973007, 0.92947, 0.835789, 0.94067, 0.817381, 0.94067,
+                0.94067, 0.94067, 0.94067, 0.94067, 0.817381],
+            "vacuum_charge": 0.0,
+            "vacuum_volume": 0.0,
+            "atomic_volume": [9.922887, 8.175158, 8.175158, 9.265802, 8.175158, 9.923233, 12.382546, 12.566972,
+                12.382546, 12.382546, 12.382546, 12.382546, 12.382546, 12.566972],
+            "charge": [12.248132, 12.26177, 12.26177, 12.600596, 12.26177, 12.248143, 7.267303, 7.256998,
+                7.267303, 7.267303, 7.267303, 7.267303, 7.267303, 7.256998],
+            "bader_version": 1.0,
+            "reference_used": True,
         }
         """
         assert set(summary) == {
@@ -128,7 +127,7 @@ def test_atom_parsing(self):
         assert len(analysis.atomic_densities) == len(analysis.chgcar.structure)
 
         assert np.sum(analysis.chgcar.data["total"]) == approx(
-            np.sum([np.sum(d["data"]) for d in analysis.atomic_densities])
+            np.sum([dct["data"] for dct in analysis.atomic_densities])
         )
 
     def test_missing_file_bader_exe_path(self):