Unreadable string concat ops to f-string (#3162)

* unreadable string concat ops to f-string

* more of the same

* fix bad return type in IMolecule.from_dict()

add literal anno for Molecule.from_str(fmt)

Author: janosh

pymatgen/analysis/bond_dissociation.py

@@ -150,20 +150,13 @@ def fragment_and_process(self, bonds):
                     # If we still have no good entries, something must have gone wrong with the calculations:
                     if len(good_entries) == 0:
                         bb = BabelMolAdaptor.from_molecule_graph(RO_frag)
-                        pbmol = bb.pybel_mol
-                        smiles = pbmol.write("smi").split()[0]
+                        pb_mol = bb.pybel_mol
+                        smiles = pb_mol.write("smi").split()[0]
                         specie = nx.get_node_attributes(self.mol_graph.graph, "specie")
                         print(
-                            "Missing ring opening fragment resulting from the breakage of "
-                            + specie[bonds[0][0]]
-                            + " "
-                            + specie[bonds[0][1]]
-                            + " bond "
-                            + str(bonds[0][0])
-                            + " "
-                            + str(bonds[0][1])
-                            + " which would yield a molecule with this SMILES string: "
-                            + smiles
+                            f"Missing ring opening fragment resulting from the breakage of {specie[bonds[0][0]]} "
+                            f"{specie[bonds[0][1]]} bond {bonds[0][0]} {bonds[0][1]} which would yield a "
+                            f"molecule with this SMILES string: {smiles}"
                         )
                     elif len(good_entries) == 1:
                         # If we have only one good entry, format it and add it to the list that will eventually return:
@@ -207,18 +200,18 @@ def fragment_and_process(self, bonds):
                 # If we're missing some of either, tell the user:
                 if len(frag1_charges_found) < len(self.expected_charges):
                     bb = BabelMolAdaptor(frags[0].molecule)
-                    pbmol = bb.pybel_mol
-                    smiles = pbmol.write("smi").split()[0]
+                    pb_mol = bb.pybel_mol
+                    smiles = pb_mol.write("smi").split()[0]
                     for charge in self.expected_charges:
                         if charge not in frag1_charges_found:
-                            print("Missing charge " + str(charge) + " for fragment " + smiles)
+                            print(f"Missing charge {charge} for fragment {smiles}")
                 if len(frag2_charges_found) < len(self.expected_charges):
                     bb = BabelMolAdaptor(frags[1].molecule)
-                    pbmol = bb.pybel_mol
-                    smiles = pbmol.write("smi").split()[0]
+                    pb_mol = bb.pybel_mol
+                    smiles = pb_mol.write("smi").split()[0]
                     for charge in self.expected_charges:
                         if charge not in frag2_charges_found:
-                            print("Missing charge " + str(charge) + " for fragment " + smiles)
+                            print(f"Missing charge {charge} for fragment {smiles}")
                 # Now we attempt to pair fragments with the right total charge, starting with only fragments with no
                 # structural change:
                 for frag1 in frag1_entries[0]:  # 0 -> no structural change
@@ -282,21 +275,16 @@ def filter_fragment_entries(self, fragment_entries):
         for entry in fragment_entries:
             # Check and make sure that PCM dielectric is consistent with principle:
             if "pcm_dielectric" in self.molecule_entry:
+                err_msg = (
+                    f"Principle molecule has a PCM dielectric of {self.molecule_entry['pcm_dielectric']}"
+                    " but a fragment entry has [[placeholder]] PCM dielectric! Please only pass fragment entries"
+                    " with PCM details consistent with the principle entry. Exiting..."
+                )
                 if "pcm_dielectric" not in entry:
-                    raise RuntimeError(
-                        "Principle molecule has a PCM dielectric of "
-                        + str(self.molecule_entry["pcm_dielectric"])
-                        + " but a fragment entry has no PCM dielectric! Please only pass fragment entries"
-                        " with PCM details consistent with the principle entry. Exiting..."
-                    )
+                    raise RuntimeError(err_msg.replace("[[placeholder]]", "no"))
                 if entry["pcm_dielectric"] != self.molecule_entry["pcm_dielectric"]:
-                    raise RuntimeError(
-                        "Principle molecule has a PCM dielectric of "
-                        + str(self.molecule_entry["pcm_dielectric"])
-                        + " but a fragment entry has a different PCM dielectric! Please only pass"
-                        " fragment entries with PCM details consistent with the principle entry."
-                        " Exiting..."
-                    )
+                    raise RuntimeError(err_msg.replace("[[placeholder]]", "a different"))
+
             # Build initial and final molgraphs:
             entry["initial_molgraph"] = MoleculeGraph.with_local_env_strategy(
                 Molecule.from_dict(entry["initial_molecule"]), OpenBabelNN()

pymatgen/analysis/ewald.py

@@ -411,22 +411,13 @@ def eta(self):
         return self._eta
 
     def __str__(self):
-        if self._compute_forces:
-            output = [
-                "Real = " + str(self.real_space_energy),
-                "Reciprocal = " + str(self.reciprocal_space_energy),
-                "Point = " + str(self.point_energy),
-                "Total = " + str(self.total_energy),
-                "Forces:\n" + str(self.forces),
-            ]
-        else:
-            output = [
-                "Real = " + str(self.real_space_energy),
-                "Reciprocal = " + str(self.reciprocal_space_energy),
-                "Point = " + str(self.point_energy),
-                "Total = " + str(self.total_energy),
-                "Forces were not computed",
-            ]
+        output = [
+            f"Real = {self.real_space_energy}",
+            f"Reciprocal = {self.reciprocal_space_energy}",
+            f"Point = {self.point_energy}",
+            f"Total = {self.total_energy}",
+            f"Forces:\n{self.forces}" if self._compute_forces else "Forces were not computed",
+        ]
         return "\n".join(output)
 
     def as_dict(self, verbosity: int = 0) -> dict:

pymatgen/analysis/fragmenter.py

@@ -105,12 +105,9 @@ def __init__(
             for level in range(depth):
                 # If on the first level, perform one level of fragmentation on the principle molecule graph:
                 if level == 0:
+                    alph_formula = self.mol_graph.molecule.composition.alphabetical_formula
                     self.fragments_by_level["0"] = self._fragment_one_level(
-                        {
-                            str(self.mol_graph.molecule.composition.alphabetical_formula)
-                            + " E"
-                            + str(len(self.mol_graph.graph.edges())): [self.mol_graph]
-                        }
+                        {f"{alph_formula} E{len(self.mol_graph.graph.edges())}": [self.mol_graph]}
                     )
                 else:
                     num_frags_prev_level = 0
@@ -183,11 +180,8 @@ def _fragment_one_level(self, old_frag_dict):
                         if self.open_rings:
                             fragments = [open_ring(old_frag, bond, self.opt_steps)]
                     for fragment in fragments:
-                        new_frag_key = (
-                            str(fragment.molecule.composition.alphabetical_formula)
-                            + " E"
-                            + str(len(fragment.graph.edges()))
-                        )
+                        alph_formula = fragment.molecule.composition.alphabetical_formula
+                        new_frag_key = f"{alph_formula} E{len(fragment.graph.edges())}"
                         proceed = True
                         if (
                             self.assume_previous_thoroughness
@@ -224,11 +218,8 @@ def _open_all_rings(self):
         we find. We also temporarily add the principle molecule graph to self.unique_fragments
         so that its rings are opened as well.
         """
-        mol_key = (
-            str(self.mol_graph.molecule.composition.alphabetical_formula)
-            + " E"
-            + str(len(self.mol_graph.graph.edges()))
-        )
+        alph_formula = self.mol_graph.molecule.composition.alphabetical_formula
+        mol_key = f"{alph_formula} E{len(self.mol_graph.graph.edges())}"
         self.all_unique_frag_dict[mol_key] = [self.mol_graph]
         new_frag_keys = {"0": []}
         new_frag_key_dict = {}
@@ -238,11 +229,8 @@ def _open_all_rings(self):
                 if ring_edges != []:
                     for bond in ring_edges[0]:
                         new_fragment = open_ring(fragment, [bond], self.opt_steps)
-                        frag_key = (
-                            str(new_fragment.molecule.composition.alphabetical_formula)
-                            + " E"
-                            + str(len(new_fragment.graph.edges()))
-                        )
+                        alph_formula = new_fragment.molecule.composition.alphabetical_formula
+                        frag_key = f"{alph_formula} E{len(new_fragment.graph.edges())}"
                         if frag_key not in self.all_unique_frag_dict:
                             if frag_key not in new_frag_keys["0"]:
                                 new_frag_keys["0"].append(copy.deepcopy(frag_key))
@@ -276,11 +264,8 @@ def _open_all_rings(self):
                     if ring_edges != []:
                         for bond in ring_edges[0]:
                             new_fragment = open_ring(fragment, [bond], self.opt_steps)
-                            frag_key = (
-                                str(new_fragment.molecule.composition.alphabetical_formula)
-                                + " E"
-                                + str(len(new_fragment.graph.edges()))
-                            )
+                            alph_formula = new_fragment.molecule.composition.alphabetical_formula
+                            frag_key = f"{alph_formula} E{len(new_fragment.graph.edges())}"
                             if frag_key not in self.all_unique_frag_dict:
                                 if frag_key not in new_frag_keys[str(idx)]:
                                     new_frag_keys[str(idx)].append(copy.deepcopy(frag_key))

pymatgen/analysis/graphs.py

@@ -2091,13 +2091,13 @@ def build_unique_fragments(self):
         frag_dict = {}
         for ii in range(1, len(self.molecule)):
             for combination in combinations(graph.nodes, ii):
-                mycomp = []
+                comp = []
                 for idx in combination:
-                    mycomp.append(str(self.molecule[idx].specie))
-                mycomp = "".join(sorted(mycomp))
+                    comp.append(str(self.molecule[idx].specie))
+                comp = "".join(sorted(comp))
                 subgraph = nx.subgraph(graph, combination)
                 if nx.is_connected(subgraph):
-                    mykey = mycomp + str(len(subgraph.edges()))
+                    mykey = comp + str(len(subgraph.edges()))
                     if mykey not in frag_dict:
                         frag_dict[mykey] = [copy.deepcopy(subgraph)]
                     else:
@@ -2142,11 +2142,8 @@ def build_unique_fragments(self):
                     )
                 )
 
-            frag_key = (
-                str(unique_mol_graph_list[0].molecule.composition.alphabetical_formula)
-                + " E"
-                + str(len(unique_mol_graph_list[0].graph.edges()))
-            )
+            alph_formula = unique_mol_graph_list[0].molecule.composition.alphabetical_formula
+            frag_key = f"{alph_formula} E{len(unique_mol_graph_list[0].graph.edges())}"
             unique_mol_graph_dict[frag_key] = copy.deepcopy(unique_mol_graph_list)
         return unique_mol_graph_dict
 

pymatgen/core/structure.py

@@ -1386,7 +1386,7 @@ def get_sites_in_sphere(
                 is included in the returned data
 
         Returns:
-            [:class:`pymatgen.core.structure.PeriodicNeighbor`]
+            PeriodicNeighbor
         """
         site_fcoords = np.mod(self.frac_coords, 1)
         neighbors: list[PeriodicNeighbor] = []
@@ -1423,7 +1423,7 @@ def get_neighbors(
                 is always included in the returned data.
 
         Returns:
-            [:class:`pymatgen.core.structure.PeriodicNeighbor`]
+            PeriodicNeighbor
         """
         return self.get_all_neighbors(r, include_index=include_index, include_image=include_image, sites=[site])[0]
 
@@ -1442,7 +1442,7 @@ def get_neighbors_old(self, site, r, include_index=False, include_image=False):
                 is included in the returned data
 
         Returns:
-            [:class:`pymatgen.core.structure.PeriodicNeighbor`]
+            PeriodicNeighbor
         """
         nn = self.get_sites_in_sphere(site.coords, r, include_index=include_index, include_image=include_image)
         return [d for d in nn if site != d[0]]
@@ -1903,7 +1903,7 @@ def get_all_neighbors_old(self, r, include_index=False, include_image=False, inc
                 data. Defaults to True.
 
         Returns:
-            [:class:`pymatgen.core.structure.PeriodicNeighbor`]
+            PeriodicNeighbor
         """
         # Use same algorithm as get_sites_in_sphere to determine supercell but
         # loop over all atoms in crystal
@@ -3160,7 +3160,7 @@ def as_dict(self):
         return d
 
     @classmethod
-    def from_dict(cls, d) -> dict:
+    def from_dict(cls, d) -> IMolecule | Molecule:
         """
         Reconstitute a Molecule object from a dict representation created using
         as_dict().
@@ -3198,7 +3198,7 @@ def get_sites_in_sphere(self, pt: ArrayLike, r: float) -> list[Neighbor]:
             r (float): Radius of sphere.
 
         Returns:
-            [:class:`pymatgen.core.structure.Neighbor`]
+            Neighbor
         """
         neighbors = []
         for i, site in enumerate(self._sites):
@@ -3217,7 +3217,7 @@ def get_neighbors(self, site: Site, r: float) -> list[Neighbor]:
             r (float): Radius of sphere.
 
         Returns:
-            [:class:`pymatgen.core.structure.Neighbor`]
+            Neighbor
         """
         nns = self.get_sites_in_sphere(site.coords, r)
         return [nn for nn in nns if nn != site]
@@ -3233,7 +3233,7 @@ def get_neighbors_in_shell(self, origin: ArrayLike, r: float, dr: float) -> list
             dr (float): Width of shell.
 
         Returns:
-            [:class:`pymatgen.core.structure.Neighbor`]
+            Neighbor
         """
         outer = self.get_sites_in_sphere(origin, r + dr)
         inner = r - dr
@@ -3429,7 +3429,9 @@ def to(self, filename: str = "", fmt: str = "") -> str | None:
         return str(writer)
 
     @classmethod
-    def from_str(cls, input_string: str, fmt: str):
+    def from_str(
+        cls, input_string: str, fmt: Literal["xyz", "gjf", "g03", "g09", "com", "inp", "json", "yaml"]
+    ) -> IMolecule | Molecule:
         """
         Reads the molecule from a string.
 
@@ -3448,21 +3450,21 @@ def from_str(cls, input_string: str, fmt: str):
         from pymatgen.io.xyz import XYZ
 
         if fmt.lower() == "xyz":
-            m = XYZ.from_str(input_string).molecule
+            mol = XYZ.from_str(input_string).molecule
         elif fmt in ["gjf", "g03", "g09", "com", "inp"]:
-            m = GaussianInput.from_str(input_string).molecule
+            mol = GaussianInput.from_str(input_string).molecule
         elif fmt == "json":
-            d = json.loads(input_string)
-            return cls.from_dict(d)
+            dct = json.loads(input_string)
+            return cls.from_dict(dct)
         elif fmt == "yaml":
             yaml = YAML()
-            d = yaml.load(input_string)
-            return cls.from_dict(d)
+            dct = yaml.load(input_string)
+            return cls.from_dict(dct)
         else:
             from pymatgen.io.babel import BabelMolAdaptor
 
-            m = BabelMolAdaptor.from_str(input_string, file_format=fmt).pymatgen_mol
-        return cls.from_sites(m)
+            mol = BabelMolAdaptor.from_str(input_string, file_format=fmt).pymatgen_mol
+        return cls.from_sites(mol)
 
     @classmethod
     def from_file(cls, filename):

pymatgen/electronic_structure/boltztrap.py

@@ -334,9 +334,9 @@ def write_struct(self, output_file):
                 ops = [[[1, 0, 0], [0, 1, 0], [0, 0, 1]]]
             f.write(f"{len(ops)}\n")
 
-            for c in ops:
-                for row in c:
-                    f.write(f"{' '.join(str(i) for i in row)}\n")
+            for op in ops:
+                for row in op:
+                    f.write(f"{' '.join(map(str, row))}\n")
 
     def write_def(self, output_file):
         """
@@ -675,12 +675,8 @@ def run(
                     logging.info(f"Could not converge with max lpfac; Decrease egrid to {self.energy_grid}")
 
             if not converged:
-                raise BoltztrapError(
-                    "Doping convergence not reached with lpfac="
-                    + str(self.lpfac)
-                    + ", energy_grid="
-                    + str(self.energy_grid)
-                )
+                lpfac, energy_grid = self.lpfac, self.energy_grid
+                raise BoltztrapError(f"Doping convergence not reached with {lpfac=}, {energy_grid=}")
 
             return path_dir
 

pymatgen/electronic_structure/plotter.py

@@ -4012,20 +4012,12 @@ def plot_fermi_surface(
 
     if energy_levels is None:
         energy_levels = [en_min + 0.01] if cbm else [en_max - 0.01]
-        print("Energy level set to: " + str(energy_levels[0]) + " eV")
+        print(f"Energy level set to: {energy_levels[0]} eV")
 
     else:
         for e in energy_levels:
             if e > en_max or e < en_min:
-                raise BoltztrapError(
-                    "energy level "
-                    + str(e)
-                    + " not in the range of possible energies: ["
-                    + str(en_min)
-                    + ", "
-                    + str(en_max)
-                    + "]"
-                )
+                raise BoltztrapError(f"energy level {e} not in the range of possible energies: [{en_min}, {en_max}]")
 
     n_surfaces = len(energy_levels)
     if colors is None: