Issue stronger warning if bader is run without the AECCARs (#3458)

* bump ruff pre-commit hook

* fix BaderAnalysis.from_path issuing missing POTCAR warning for missing AECCAR0/2 and vice versa

Author: janosh

.pre-commit-config.yaml

@@ -8,7 +8,7 @@ ci:
 
 repos:
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.1.3
+    rev: v0.1.4
     hooks:
       - id: ruff
         args: [--fix]

pymatgen/command_line/bader_caller.py

@@ -371,16 +371,15 @@ def from_path(cls, path: str, suffix: str = "") -> BaderAnalysis:
         to perform Bader analysis.
 
         Args:
-            path (str): Name of directory where VASP output files are
-                stored.
+            path (str): Name of directory where VASP output files are stored.
             suffix (str): specific suffix to look for (e.g. '.relax1' for 'CHGCAR.relax1.gz').
 
         Returns:
             BaderAnalysis
         """
 
         def _get_filepath(filename):
-            name_pattern = filename + suffix + "*" if filename != "POTCAR" else filename + "*"
+            name_pattern = f"{filename}{suffix}*" if filename != "POTCAR" else f"{filename}*"
             paths = glob(f"{path}/{name_pattern}")
             fpath = ""
             if len(paths) >= 1:
@@ -394,10 +393,10 @@ def _get_filepath(filename):
                 fpath = paths[0]
             else:
                 msg = f"Could not find {filename!r}"
-                if filename in ["AECCAR0", "AECCAR2"]:
-                    msg += ", cannot calculate charge transfer."
+                if filename in ("AECCAR0", "AECCAR2"):
+                    msg += ", interpret Bader results with severe caution."
                 elif filename == "POTCAR":
-                    msg += ", interpret Bader results with caution."
+                    msg += ", cannot calculate charge transfer."
                 warnings.warn(msg)
             return fpath
 
@@ -440,10 +439,9 @@ def bader_analysis_from_path(path, suffix=""):
         summary dict
     """
 
-    def _get_filepath(filename, warning, path=path, suffix=suffix):
+    def _get_filepath(filename, path=path, suffix=suffix):
         paths = glob(f"{path}/{filename}{suffix}*")
-        if not paths:
-            warnings.warn(warning)
+        if len(paths) == 0:
             return None
         if len(paths) > 1:
             # using reverse=True because, if multiple files are present,
@@ -457,13 +455,19 @@ def _get_filepath(filename, warning, path=path, suffix=suffix):
     chgcar_path = _get_filepath("CHGCAR", "Could not find CHGCAR!")
     chgcar = Chgcar.from_file(chgcar_path)
 
-    aeccar0_path = _get_filepath("AECCAR0", "Could not find AECCAR0, interpret Bader results with caution.")
+    aeccar0_path = _get_filepath("AECCAR0")
+    if not aeccar0_path:
+        warnings.warn("Could not find AECCAR0, interpret Bader results with severe caution!")
     aeccar0 = Chgcar.from_file(aeccar0_path) if aeccar0_path else None
 
-    aeccar2_path = _get_filepath("AECCAR2", "Could not find AECCAR2, interpret Bader results with caution.")
+    aeccar2_path = _get_filepath("AECCAR2")
+    if not aeccar2_path:
+        warnings.warn("Could not find AECCAR2, interpret Bader results with severe caution!")
     aeccar2 = Chgcar.from_file(aeccar2_path) if aeccar2_path else None
 
-    potcar_path = _get_filepath("POTCAR", "Could not find POTCAR, cannot calculate charge transfer.")
+    potcar_path = _get_filepath("POTCAR")
+    if not potcar_path:
+        warnings.warn("Could not find POTCAR, cannot calculate charge transfer.")
     potcar = Potcar.from_file(potcar_path) if potcar_path else None
 
     return bader_analysis_from_objects(chgcar, potcar, aeccar0, aeccar2)

pymatgen/io/abinit/netcdf.py

@@ -36,25 +36,25 @@
 __date__ = "Feb 21, 2013M"
 
 
-def _asreader(file, cls):
-    closeit = False
+def _as_reader(file, cls):
+    close_it = False
     if not isinstance(file, cls):
-        file, closeit = cls(file), True
-    return file, closeit
+        file, close_it = cls(file), True
+    return file, close_it
 
 
 def as_ncreader(file):
     """
     Convert file into a NetcdfReader instance.
-    Returns reader, closeit where closeit is set to True
+    Returns reader, close_it where close_it is set to True
     if we have to close the file before leaving the procedure.
     """
-    return _asreader(file, NetcdfReader)
+    return _as_reader(file, NetcdfReader)
 
 
 def as_etsfreader(file):
     """Return an EtsfReader. Accepts filename or EtsfReader."""
-    return _asreader(file, EtsfReader)
+    return _as_reader(file, EtsfReader)
 
 
 class NetcdfReaderError(Exception):
@@ -302,7 +302,7 @@ def structure_from_ncdata(ncdata, site_properties=None, cls=Structure):
         site_properties: Dictionary with site properties.
         cls: The Structure class to instantiate.
     """
-    ncdata, closeit = as_ncreader(ncdata)
+    ncdata, close_it = as_ncreader(ncdata)
 
     # TODO check whether atomic units are used
     lattice = ArrayWithUnit(ncdata.read_value("primitive_vectors"), "bohr").to("ang")
@@ -337,7 +337,7 @@ def structure_from_ncdata(ncdata, site_properties=None, cls=Structure):
     except ImportError:
         pass
 
-    if closeit:
+    if close_it:
         ncdata.close()
 
     return structure

pymatgen/io/vasp/inputs.py

@@ -2696,5 +2696,5 @@ def run_vasp(
         vasp_cmd = [os.path.expanduser(os.path.expandvars(t)) for t in vasp_cmd]
         if not vasp_cmd:
             raise RuntimeError("You need to supply vasp_cmd or set the PMG_VASP_EXE in .pmgrc.yaml to run VASP.")
-        with cd(run_dir), open(output_file, "w") as f_std, open(err_file, "w", buffering=1) as f_err:
-            subprocess.check_call(vasp_cmd, stdout=f_std, stderr=f_err)
+        with cd(run_dir), open(output_file, "w") as stdout_file, open(err_file, "w", buffering=1) as stderr_file:
+            subprocess.check_call(vasp_cmd, stdout=stdout_file, stderr=stderr_file)