Clean up core.surface comments and docstrings

pymatgen/core/structure.py

@@ -578,8 +578,7 @@ def add_oxidation_state_by_element(self, oxidation_states: dict[str, float]) ->
         Returns:
             SiteCollection: self with oxidation states.
         """
-        missing = {el.symbol for el in self.composition} - {*oxidation_states}
-        if missing:
+        if missing := {el.symbol for el in self.composition} - {*oxidation_states}:
             raise ValueError(f"Oxidation states not specified for all elements, {missing=}")
         for site in self:
             new_sp = {}
@@ -792,7 +791,7 @@ def _relax(
         # UIP=universal interatomic potential
         run_uip = isinstance(calculator, str) and calculator.lower() in ("m3gnet", "chgnet")
 
-        calc_params = dict(stress_weight=stress_weight) if not is_molecule else {}
+        calc_params = {} if is_molecule else dict(stress_weight=stress_weight)
         calculator = self._prep_calculator(calculator, **calc_params)
 
         # check str is valid optimizer key
@@ -1031,7 +1030,7 @@ def from_sites(
         Returns:
             (Structure) Note that missing properties are set as None.
         """
-        if len(sites) < 1:
+        if not sites:
             raise ValueError(f"You need at least 1 site to construct a {cls.__name__}")
         prop_keys: list[str] = []
         props = {}
@@ -1133,9 +1132,7 @@ def from_spacegroup(
         if len(species) != len(coords):
             raise ValueError(f"Supplied species and coords lengths ({len(species)} vs {len(coords)}) are different!")
 
-        frac_coords = (
-            np.array(coords, dtype=np.float64) if not coords_are_cartesian else latt.get_fractional_coords(coords)
-        )
+        frac_coords = latt.get_fractional_coords(coords) if coords_are_cartesian else np.array(coords, dtype=np.float64)
 
         props = {} if site_properties is None else site_properties
 
@@ -1236,7 +1233,7 @@ def from_magnetic_spacegroup(
             if len(var) != len(species):
                 raise ValueError(f"Length mismatch: len({name})={len(var)} != {len(species)=}")
 
-        frac_coords = coords if not coords_are_cartesian else latt.get_fractional_coords(coords)
+        frac_coords = latt.get_fractional_coords(coords) if coords_are_cartesian else coords
 
         all_sp: list[str | Element | Species | DummySpecies | Composition] = []
         all_coords: list[list[float]] = []
@@ -2237,7 +2234,7 @@ def interpolate(
         if not (interpolate_lattices or self.lattice == end_structure.lattice):
             raise ValueError("Structures with different lattices!")
 
-        images = np.arange(nimages + 1) / nimages if not isinstance(nimages, collections.abc.Iterable) else nimages
+        images = nimages if isinstance(nimages, collections.abc.Iterable) else np.arange(nimages + 1) / nimages
 
         # Check that both structures have the same species
         for idx, site in enumerate(self):
@@ -2638,7 +2635,7 @@ def as_dict(self, verbosity=1, fmt=None, **kwargs) -> dict[str, Any]:
             JSON-serializable dict representation.
         """
         if fmt == "abivars":
-            """Returns a dictionary with the ABINIT variables."""
+            # Returns a dictionary with the ABINIT variables
             from pymatgen.io.abinit.abiobjects import structure_to_abivars
 
             return structure_to_abivars(self, **kwargs)
@@ -3502,7 +3499,7 @@ def get_boxed_structure(
                         z_max, z_min = max(new_coords[:, 2]), min(new_coords[:, 2])
                         if x_max > a or x_min < 0 or y_max > b or y_min < 0 or z_max > c or z_min < 0:
                             raise ValueError("Molecule crosses boundary of box")
-                    if len(all_coords) == 0:
+                    if not all_coords:
                         break
                     distances = lattice.get_all_distances(
                         lattice.get_fractional_coords(new_coords),
@@ -3587,15 +3584,15 @@ def to(self, filename: str = "", fmt: str = "") -> str | None:
         writer: Any
         if fmt == "xyz" or fnmatch(filename.lower(), "*.xyz*"):
             writer = XYZ(self)
-        elif any(fmt == ext or fnmatch(filename.lower(), f"*.{ext}*") for ext in ["gjf", "g03", "g09", "com", "inp"]):
+        elif any(fmt == ext or fnmatch(filename.lower(), f"*.{ext}*") for ext in ("gjf", "g03", "g09", "com", "inp")):
             writer = GaussianInput(self)
         elif fmt == "json" or fnmatch(filename, "*.json*") or fnmatch(filename, "*.mson*"):
             json_str = json.dumps(self.as_dict())
             if filename:
                 with zopen(filename, mode="wt", encoding="utf8") as file:
                     file.write(json_str)
             return json_str
-        elif fmt in ("yaml", "yml") or fnmatch(filename, "*.yaml*") or fnmatch(filename, "*.yml*"):
+        elif fmt in {"yaml", "yml"} or fnmatch(filename, "*.yaml*") or fnmatch(filename, "*.yml*"):
             yaml = YAML()
             str_io = StringIO()
             yaml.dump(self.as_dict(), str_io)
@@ -3685,8 +3682,7 @@ def from_file(cls, filename):
             return cls.from_str(contents, fmt="yaml")
         from pymatgen.io.babel import BabelMolAdaptor
 
-        match = re.search(r"\.(pdb|mol|mdl|sdf|sd|ml2|sy2|mol2|cml|mrv)", filename.lower())
-        if match:
+        if match := re.search(r"\.(pdb|mol|mdl|sdf|sd|ml2|sy2|mol2|cml|mrv)", filename.lower()):
             new = BabelMolAdaptor.from_file(filename, match.group(1)).pymatgen_mol
             new.__class__ = cls
             return new
@@ -3994,13 +3990,14 @@ def substitute(self, index: int, func_group: IMolecule | Molecule | str, bond_or
 
         # Pass value of functional group--either from user-defined or from
         # functional.json
-        if not isinstance(func_group, Molecule):
+        if isinstance(func_group, Molecule):
+            fgroup = func_group
+
+        else:
             # Check to see whether the functional group is in database.
             if func_group not in FunctionalGroups:
                 raise RuntimeError("Can't find functional group in list. Provide explicit coordinate instead")
             fgroup = FunctionalGroups[func_group]
-        else:
-            fgroup = func_group
 
         # If a bond length can be found, modify func_grp so that the X-group
         # bond length is equal to the bond length.
@@ -4478,22 +4475,23 @@ def from_prototype(cls, prototype: str, species: Sequence, **kwargs) -> Structur
                 return Structure.from_spacegroup(
                     "Pm-3m", Lattice.cubic(kwargs["a"]), species, [[0, 0, 0], [0.5, 0.5, 0.5], [0.5, 0.5, 0]]
                 )
-            if prototype in ("cscl"):
+            if prototype == "cscl":
                 return Structure.from_spacegroup(
                     "Pm-3m", Lattice.cubic(kwargs["a"]), species, [[0, 0, 0], [0.5, 0.5, 0.5]]
                 )
-            if prototype in ("fluorite", "caf2"):
+            if prototype in {"fluorite", "caf2"}:
                 return Structure.from_spacegroup(
                     "Fm-3m", Lattice.cubic(kwargs["a"]), species, [[0, 0, 0], [1 / 4, 1 / 4, 1 / 4]]
                 )
-            if prototype in ("antifluorite"):
+            if prototype == "antifluorite":
                 return Structure.from_spacegroup(
                     "Fm-3m", Lattice.cubic(kwargs["a"]), species, [[1 / 4, 1 / 4, 1 / 4], [0, 0, 0]]
                 )
-            if prototype in ("zincblende"):
+            if prototype == "zincblende":
                 return Structure.from_spacegroup(
                     "F-43m", Lattice.cubic(kwargs["a"]), species, [[0, 0, 0], [1 / 4, 1 / 4, 3 / 4]]
                 )
+
         except KeyError as exc:
             raise ValueError(f"Required parameter {exc} not specified as a kwargs!") from exc
         raise ValueError(f"Unsupported {prototype=}!")
@@ -4978,5 +4976,5 @@ class StructureError(Exception):
     """
 
 
-with open(os.path.join(os.path.dirname(__file__), "func_groups.json")) as file:
+with open(os.path.join(os.path.dirname(__file__), "func_groups.json"), encoding="utf-8") as file:
     FunctionalGroups = {k: Molecule(v["species"], v["coords"]) for k, v in json.load(file).items()}

pymatgen/core/surface.py

@@ -86,7 +86,6 @@ def __init__(
         coords_are_cartesian=False,
         site_properties=None,
         energy=None,
-        properties=None,
     ) -> None:
         """Makes a Slab structure, a structure object with additional information
         and methods pertaining to slabs.
@@ -132,8 +131,6 @@ def __init__(
                 have to be the same length as the atomic species and
                 fractional_coords. Defaults to None for no properties.
             energy (float): A value for the energy.
-            properties (dict): dictionary containing properties associated
-                with the whole slab.
         """
         self.oriented_unit_cell = oriented_unit_cell
         self.miller_index = tuple(miller_index)
@@ -449,9 +446,6 @@ def add_adsorbate_atom(self, indices, specie, distance) -> Slab:
         return self
 
     def __str__(self) -> str:
-        def to_str(x) -> str:
-            return f"{x:0.6f}"
-
         comp = self.composition
         outs = [
             f"Slab Summary ({comp.formula})",
@@ -501,7 +495,6 @@ def from_dict(cls, dct: dict) -> Slab:  # type: ignore[override]
             scale_factor=dct["scale_factor"],
             site_properties=struct.site_properties,
             energy=dct["energy"],
-            properties=dct.get("properties"),
         )
 
     def get_surface_sites(self, tag=False):
@@ -1246,7 +1239,7 @@ def nonstoichiometric_symmetrized_slab(self, init_slab):
                 # surfaces are symmetric or the number of sites removed has
                 # exceeded 10 percent of the original slab
 
-                c_dir = [site[2] for idx, site in enumerate(slab.frac_coords)]
+                c_dir = [site[2] for site in slab.frac_coords]
 
                 if top:
                     slab.remove_sites([c_dir.index(max(c_dir))])
@@ -1267,7 +1260,7 @@ def nonstoichiometric_symmetrized_slab(self, init_slab):
 
 
 module_dir = os.path.dirname(os.path.abspath(__file__))
-with open(f"{module_dir}/reconstructions_archive.json") as data_file:
+with open(f"{module_dir}/reconstructions_archive.json", encoding="utf-8") as data_file:
     reconstructions_archive = json.load(data_file)
 
 
@@ -1633,10 +1626,10 @@ def hkl_transformation(transf, miller_index):
     """
     # Get a matrix of whole numbers (ints)
 
-    def lcm(a, b):
+    def _lcm(a, b):
         return a * b // math.gcd(a, b)
 
-    reduced_transf = reduce(lcm, [int(1 / i) for i in itertools.chain(*transf) if i != 0]) * transf
+    reduced_transf = reduce(_lcm, [int(1 / i) for i in itertools.chain(*transf) if i != 0]) * transf
     reduced_transf = reduced_transf.astype(int)
 
     # perform the transformation
@@ -1825,7 +1818,7 @@ def get_slab_regions(slab, blength=3.5):
         for site in slab:
             if all(nn.index not in all_indices for nn in slab.get_neighbors(site, blength)):
                 upper_fcoords.append(site.frac_coords[2])
-        coords = copy.copy(last_fcoords) if not fcoords else copy.copy(fcoords)
+        coords = copy.copy(fcoords) if fcoords else copy.copy(last_fcoords)
         min_top = slab[last_indices[coords.index(min(coords))]].frac_coords[2]
         ranges = [[0, max(upper_fcoords)], [min_top, 1]]
     else:

tests/apps/borg/test_queen.py

@@ -2,6 +2,8 @@
 
 import unittest
 
+from pytest import approx
+
 from pymatgen.apps.borg.hive import VaspToComputedEntryDrone
 from pymatgen.apps.borg.queen import BorgQueen
 from pymatgen.util.testing import TEST_FILES_DIR
@@ -20,7 +22,7 @@ def test_get_data(self):
         queen = BorgQueen(drone, TEST_DIR, 1)
         data = queen.get_data()
         assert len(data) == 1
-        assert data[0].energy == 0.5559329
+        assert data[0].energy == approx(0.5559329, 1e-4)
 
     def test_load_data(self):
         drone = VaspToComputedEntryDrone()