Merge branch 'master' of github.com:materialsproject/pymatgen

Author: Shyue Ping Ong

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/analysis/interface_reactions.py

@@ -585,8 +585,7 @@ def labels(self):
                3: 'x= 1 energy = 0 O2 -> O2'}.
         """
         return {
-            j: "x= " + str(round(x, 4)) + " energy in eV/atom = " + str(round(energy, 4)) + " " + str(reaction)
-            for j, x, energy, reaction, _ in self.get_kinks()
+            j: f"x= {x:.4} energy in eV/atom = {energy:.4} {reaction}" for j, x, energy, reaction, _ in self.get_kinks()
         }
 
     @property
@@ -598,7 +597,7 @@ def minimum(self):
         Returns:
             Tuple (x_min, E_min).
         """
-        return min(((x, energy) for _, x, energy, _, _ in self.get_kinks()), key=lambda i: i[1])
+        return min(((x, energy) for _, x, energy, _, _ in self.get_kinks()), key=lambda tup: tup[1])
 
     @property
     def products(self):

pymatgen/analysis/magnetism/heisenberg.py

@@ -253,8 +253,8 @@ def _get_exchange_df(self):
         # Get labels of unique NN interactions
         for k0, v0 in nn_interactions.items():
             for idx, j in v0.items():  # i and j indices
-                c = str(idx) + "-" + str(j) + "-" + str(k0)
-                c_rev = str(j) + "-" + str(idx) + "-" + str(k0)
+                c = f"{idx}-{j}-{k0}"
+                c_rev = f"{j}-{idx}-{k0}"
                 if c not in columns and c_rev not in columns:
                     columns.append(c)
 
@@ -311,8 +311,8 @@ def _get_exchange_df(self):
                             order = "-nnn"
                         elif abs(dist - dists["nnnn"]) <= tol:
                             order = "-nnnn"
-                        j_ij = str(i_index) + "-" + str(j_index) + order
-                        j_ji = str(j_index) + "-" + str(i_index) + order
+                        j_ij = f"{i_index}-{j_index}{order}"
+                        j_ji = f"{j_index}-{i_index}{order}"
 
                         if j_ij in ex_mat.columns:
                             ex_row.loc[sgraph_index, j_ij] -= s_i * s_j
@@ -611,8 +611,8 @@ def _get_j_exc(self, i, j, dist):
         elif abs(dist - self.dists["nnnn"]) <= self.tol:
             order = "-nnnn"
 
-        j_ij = str(i_index) + "-" + str(j_index) + order
-        j_ji = str(j_index) + "-" + str(i_index) + order
+        j_ij = f"{i_index}-{j_index}{order}"
+        j_ji = f"{j_index}-{i_index}{order}"
 
         if j_ij in self.ex_params:
             j_exc = self.ex_params[j_ij]
@@ -631,7 +631,7 @@ def get_heisenberg_model(self):
         """Save results of mapping to a HeisenbergModel object.
 
         Returns:
-            hmodel (HeisenbergModel): MSONable object.
+            HeisenbergModel: MSONable object.
         """
         # Original formula unit with nonmagnetic ions
         hm_formula = str(self.ordered_structures_[0].composition.reduced_formula)
@@ -981,8 +981,8 @@ def _get_j_exc(self, i, j, dist):
         elif abs(dist - self.dists["nnnn"]) <= self.tol:
             order = "-nnnn"
 
-        j_ij = str(i_index) + "-" + str(j_index) + order
-        j_ji = str(j_index) + "-" + str(i_index) + order
+        j_ij = f"{i_index}-{j_index}{order}"
+        j_ji = f"{j_index}-{i_index}{order}"
 
         if j_ij in self.ex_params:
             j_exc = self.ex_params[j_ij]

pymatgen/analysis/surface_analysis.py

@@ -216,7 +216,7 @@ def surface_energy(self, ucell_entry, ref_entries=None):
         # from each element with an existing ref_entry.
         bulk_energy, gbulk_eqn = 0, 0
         for el, ref in ref_entries_dict.items():
-            N, delu = self.composition.as_dict()[el], Symbol("delu_" + str(el))
+            N, delu = self.composition.as_dict()[el], Symbol(f"delu_{el}")
             if el in ucell_comp.as_dict():
                 gbulk_eqn += ucell_reduced_comp[el] * (delu + ref.energy_per_atom)
             bulk_energy += N * (Symbol("delu_" + el) + ref.energy_per_atom)

pymatgen/analysis/tests/pdentries_test.csv renamed to pymatgen/analysis/tests/pd_entries_test.csv

@@ -30,6 +30,7 @@
 from pymatgen.core.periodic_table import DummySpecies, Element
 from pymatgen.entries.computed_entries import ComputedEntry
 from pymatgen.entries.entry_tools import EntrySet
+from pymatgen.util.testing import PymatgenTest
 
 module_dir = Path(__file__).absolute().parent
 
@@ -93,7 +94,7 @@ def test_str(self):
         assert str(pde) == "PDEntry : Li1 Fe1 O2 with energy = 53.0000"
 
     def test_read_csv(self):
-        entries = EntrySet.from_csv(module_dir / "pdentries_test.csv")
+        entries = EntrySet.from_csv(module_dir / "pd_entries_test.csv")
         assert entries.chemsys == {"Li", "Fe", "O"}, "Wrong elements!"
         assert len(entries) == 490, "Wrong number of entries!"
 
@@ -147,9 +148,9 @@ def test_normalize(self):
         assert norm_entry.composition == expected_comp, "Wrong composition!"
 
 
-class PhaseDiagramTest(unittest.TestCase):
+class PhaseDiagramTest(PymatgenTest):
     def setUp(self):
-        self.entries = EntrySet.from_csv(module_dir / "pdentries_test.csv")
+        self.entries = EntrySet.from_csv(module_dir / "pd_entries_test.csv")
         self.pd = PhaseDiagram(self.entries)
         warnings.simplefilter("ignore")
 
@@ -590,8 +591,10 @@ def test_to_from_dict(self):
         assert isinstance(pd.to_json(), str)
 
     def test_read_json(self):
-        dumpfn(self.pd, "pd.json")
-        loadfn("pd.json")
+        dumpfn(self.pd, f"{self.tmp_path}/pd.json")
+        pd = loadfn(f"{self.tmp_path}/pd.json")
+        assert isinstance(pd, PhaseDiagram)
+        assert {*pd.as_dict()} == {*self.pd.as_dict()}
 
     def test_el_refs(self):
         # Create an imitation of pre_computed phase diagram with el_refs keys being
@@ -611,7 +614,7 @@ def test_val_err_on_no_entries(self):
 
 class GrandPotentialPhaseDiagramTest(unittest.TestCase):
     def setUp(self):
-        self.entries = EntrySet.from_csv(module_dir / "pdentries_test.csv")
+        self.entries = EntrySet.from_csv(module_dir / "pd_entries_test.csv")
         self.pd = GrandPotentialPhaseDiagram(self.entries, {Element("O"): -5})
         self.pd6 = GrandPotentialPhaseDiagram(self.entries, {Element("O"): -6})
 
@@ -645,7 +648,7 @@ def test_str(self):
 
 class CompoundPhaseDiagramTest(unittest.TestCase):
     def setUp(self):
-        self.entries = EntrySet.from_csv(module_dir / "pdentries_test.csv")
+        self.entries = EntrySet.from_csv(module_dir / "pd_entries_test.csv")
         self.pd = CompoundPhaseDiagram(self.entries, [Composition("Li2O"), Composition("Fe2O3")])
 
     def test_stable_entries(self):
@@ -843,7 +846,7 @@ def test_formula(self):
 
 class PDPlotterTest(unittest.TestCase):
     def setUp(self):
-        entries = list(EntrySet.from_csv(os.path.join(module_dir, "pdentries_test.csv")))
+        entries = list(EntrySet.from_csv(os.path.join(module_dir, "pd_entries_test.csv")))
 
         elemental_entries = [e for e in entries if e.composition.elements == [Element("Li")]]
         self.pd_unary = PhaseDiagram(elemental_entries)