docstring and type tweaks for io.packmol

src/pymatgen/io/packmol.py

@@ -1,20 +1,17 @@
 """
-This module provides a pymatgen I/O interface to packmol.
+This module provides a pymatgen I/O interface to PACKMOL.
 
-This adopts the minimal core I/O interface (see pymatgen/io/core).
-In this case, only a two classes are used. PackmolSet(InputSet) is the container
-class that provides a run() method for running packmol locally.
+- PackmolSet provides a "run" method to run PACKMOL locally.
 
-PackmolBoxGen(InputGenerator) provides a recipe for packing molecules into a
-box, which returns a PackmolSet object.
+- PackmolBoxGen provides "get_input_set" for packing molecules into a box,
+which returns a PackmolSet object.
 
-For the run() method to work, you need to install the packmol package
-See http://m3g.iqm.unicamp.br/packmol or
-http://leandro.iqm.unicamp.br/m3g/packmol/home.shtml
-for download and setup instructions. Note that packmol versions prior to 20.3.0
-do not support paths with spaces.
-After installation, you may need to manually add the path of the packmol
+For the run() method to work, you need to install the PACKMOL package.
+See http://m3g.iqm.unicamp.br/packmol for download and setup instructions.
+After installation, you may need to add the path of the PACKMOL
 executable to the PATH environment variable.
+
+Note that PACKMOL versions prior to 20.3.0 do not support paths with spaces.
 """
 
 from __future__ import annotations
@@ -41,18 +38,18 @@ class that provides a run() method for running packmol locally.
 
 
 class PackmolSet(InputSet):
-    """InputSet for the Packmol software. This class defines several attributes related to."""
+    """InputSet for the PACKMOL software. This class defines several attributes related to."""
 
-    def run(self, path: PathLike, timeout=30):
-        """Run packmol and write out the packed structure.
+    def run(self, path: PathLike, timeout: float = 30) -> None:
+        """Run PACKMOL and write out the packed structure.
 
         Args:
-            path: The path in which packmol input files are located.
-            timeout: Timeout in seconds.
+            path (PathLike): The path in which packmol input files are located.
+            timeout (float): Timeout in seconds.
 
         Raises:
-            ValueError if packmol does not succeed in packing the box.
-            TimeoutExpiredError if packmold does not finish within the timeout.
+            ValueError: if packmol does not succeed in packing the box.
+            TimeoutExpiredError: if packmol does not finish within the timeout.
         """
         wd = os.getcwd()
         if not which("packmol"):
@@ -70,9 +67,9 @@ def run(self, path: PathLike, timeout=30):
                 timeout=timeout,
                 capture_output=True,
             )
-            # this workaround is needed because packmol can fail to find
-            # a solution but still return a zero exit code
-            # see https://github.com/m3g/packmol/issues/28
+            # This workaround is needed because packmol can fail to find
+            # a solution but still return a zero exit code.
+            # See https://github.com/m3g/packmol/issues/28
             if "ERROR" in proc.stdout.decode():
                 if "Could not open file." in proc.stdout.decode():
                     raise ValueError(
@@ -81,10 +78,11 @@ def run(self, path: PathLike, timeout=30):
                     )
                 msg = proc.stdout.decode().split("ERROR")[-1]
                 raise ValueError(f"Packmol failed with return code 0 and stdout: {msg}")
+
         except subprocess.CalledProcessError as exc:
             raise ValueError(f"Packmol failed with error code {exc.returncode} and stderr: {exc.stderr}") from exc
         else:
-            with open(Path(path, self.stdoutfile), mode="w") as out:
+            with open(Path(path, self.stdoutfile), mode="w", encoding="utf-8") as out:
                 out.write(proc.stdout.decode())
         finally:
             os.chdir(wd)
@@ -120,12 +118,12 @@ def __init__(
         like filenames, random seed, tolerance, etc.
 
         Args:
-            tolerance: Tolerance for packmol, in Å.
-            seed: Random seed for packmol. Use a value of 1 (default) for deterministic
+            tolerance (float): Tolerance for packmol, in Å.
+            seed (int): Random seed for packmol. Use 1 (default) for deterministic
                 output, or -1 to generate a new random seed from the current time.
-            inputfile: Path to the input file. Default to 'packmol.inp'.
-            outputfile: Path to the output file. Default to 'output.xyz'.
-            stdoutfile: Path to the file where stdout will be recorded. Default to 'packmol.stdout'
+            inputfile (PathLike): Path to the input file. Default to "packmol.inp".
+            outputfile (PathLike): Path to the output file. Default to "packmol_out.xyz".
+            stdoutfile (PathLike): Path to the file where stdout will be recorded. Default to "packmol.stdout".
         """
         self.inputfile = inputfile
         self.outputfile = outputfile
@@ -142,18 +140,19 @@ def get_input_set(
         """Generate a Packmol InputSet for a set of molecules.
 
         Args:
-            molecules: A list of dict containing information about molecules to pack
+            molecules (list[dict]): Information about molecules to pack
                 into the box. Each dict requires three keys:
-                    1. "name" - the structure name
-                    2. "number" - the number of that molecule to pack into the box
-                    3. "coords" - Coordinates in the form of either a Molecule object or
-                        a path to a file.
-
-        Example:
-                    {"name": "water",
-                     "number": 500,
-                     "coords": "/path/to/input/file.xyz"}
-            box: A list of box dimensions xlo, ylo, zlo, xhi, yhi, zhi, in Å. If set to None
+                    1. "name" - the structure name.
+                    2. "number" - the number of that molecule to pack into the box.
+                    3. "coords" - Coordinates in the form of either a Molecule
+                        object or a path to a file.
+                Example:
+                    {
+                        "name": "water",
+                        "number": 500,
+                        "coords": "/path/to/input/file.xyz",
+                    }
+            box (list[float]): Box dimensions xlo, ylo, zlo, xhi, yhi, zhi, in Å. If set to None
                 (default), pymatgen will estimate the required box size based on the volumes of
                 the provided molecules.
         """
@@ -181,37 +180,37 @@ def get_input_set(
         if box:
             box_list = " ".join(str(i) for i in box)
         else:
-            # estimate the total volume of all molecules in cubic Å
+            # Estimate the total volume of all molecules in cubic Å
             net_volume = 0.0
-            for d in molecules:
-                mol = d["coords"] if isinstance(d["coords"], Molecule) else Molecule.from_file(d["coords"])
+            for dct in molecules:
+                mol = dct["coords"] if isinstance(dct["coords"], Molecule) else Molecule.from_file(dct["coords"])
 
                 if mol is None:
                     raise ValueError("Molecule cannot be None.")
-                # pad the calculated length by an amount related to the tolerance parameter
+                # Pad the calculated length by an amount related to the tolerance parameter
                 # the amount to add was determined arbitrarily
                 length = (
                     max(np.max(mol.cart_coords[:, i]) - np.min(mol.cart_coords[:, i]) for i in range(3))
                     + self.tolerance
                 )
-                net_volume += (length**3.0) * float(d["number"])
+                net_volume += (length**3.0) * float(dct["number"])
             box_length = net_volume ** (1 / 3)
             print(f"Auto determined box size is {box_length:.1f} Å per side.")
             box_list = f"0.0 0.0 0.0 {box_length:.1f} {box_length:.1f} {box_length:.1f}"
 
-        for d in molecules:
+        for dct in molecules:
             mol = None
-            if isinstance(d["coords"], str):
-                mol = Molecule.from_file(d["coords"])
-            elif isinstance(d["coords"], Path):
-                mol = Molecule.from_file(str(d["coords"]))
-            elif isinstance(d["coords"], Molecule):
-                mol = d["coords"]
+            if isinstance(dct["coords"], str):
+                mol = Molecule.from_file(dct["coords"])
+            elif isinstance(dct["coords"], Path):
+                mol = Molecule.from_file(str(dct["coords"]))
+            elif isinstance(dct["coords"], Molecule):
+                mol = dct["coords"]
 
             if mol is None:
                 raise ValueError("Molecule cannot be None.")
 
-            fname = f"packmol_{d['name']}.xyz"
+            fname = f"packmol_{dct['name']}.xyz"
             mapping[fname] = mol.to(fmt="xyz")
             if " " in str(fname):
                 # NOTE - double quotes are deliberately used inside the f-string here, do not change
@@ -220,7 +219,7 @@ def get_input_set(
                 # fmt: on
             else:
                 file_contents += f"structure {fname}\n"
-            file_contents += f"  number {d['number']}\n"
+            file_contents += f"  number {dct['number']}\n"
             file_contents += f"  inside box {box_list}\n"
             file_contents += "end structure\n\n"