add TODO comment re overwriting blocks with equal headers in CifFile.…

…from_str()

pymatgen/apps/battery/analyzer.py

@@ -25,22 +25,22 @@
 class BatteryAnalyzer:
     """A suite of methods for starting with an oxidized structure and determining its potential as a battery."""
 
-    def __init__(self, struc_oxid, working_ion="Li", oxi_override=None):
+    def __init__(self, struct_oxid, working_ion="Li", oxi_override=None):
         """Pass in a structure for analysis.
 
         Arguments:
-            struc_oxid: a Structure object; oxidation states *must* be assigned for this structure; disordered
+            struct_oxid: a Structure object; oxidation states *must* be assigned for this structure; disordered
                 structures should be OK
             working_ion: a String symbol or Element for the working ion.
             oxi_override: a dict of String element symbol, Integer oxidation state pairs.
                 by default, H, C, N, O, F, S, Cl, Se, Br, Te, I are considered anions.
         """
-        for site in struc_oxid:
+        for site in struct_oxid:
             if not hasattr(site.specie, "oxi_state"):
                 raise ValueError("BatteryAnalyzer requires oxidation states assigned to structure!")
-        self.struc_oxid = struc_oxid
+        self.struct_oxid = struct_oxid
         self.oxi_override = oxi_override or {}
-        self.comp = self.struc_oxid.composition  # shortcut for later
+        self.comp = self.struct_oxid.composition  # shortcut for later
 
         if not isinstance(working_ion, Element):
             self.working_ion = Element(working_ion)
@@ -152,7 +152,7 @@ def get_max_capvol(self, remove=True, insert=True, volume=None):
         Returns:
             max vol capacity in mAh/cc
         """
-        vol = volume or self.struc_oxid.volume
+        vol = volume or self.struct_oxid.volume
         return self._get_max_cap_ah(remove, insert) * 1000 * 1e24 / (vol * const.N_A)
 
     def get_removals_int_oxid(self):

pymatgen/io/cif.py

@@ -265,6 +265,8 @@ def from_str(cls, string) -> CifFile:
             if "powder_pattern" in re.split(r"\n", block_str, maxsplit=1)[0]:
                 continue
             block = CifBlock.from_str("data_" + block_str)
+            # TODO (@janosh, 2023-10-11) multiple CIF blocks with equal header will overwrite each other,
+            # latest taking precedence. maybe something to fix and test e.g. in test_cif_writer_write_file
             dct[block.header] = block
 
         return cls(dct, string)
@@ -1493,8 +1495,6 @@ def write_file(self, filename: str | Path, mode: Literal["w", "a", "wt", "at"] =
         """Write the CIF file."""
         with zopen(filename, mode=mode) as file:
             file.write(str(self))
-            if mode in ["a", "at"]:
-                file.write("\n\n")
 
 
 def str2float(text):

pymatgen/transformations/advanced_transformations.py

@@ -1962,36 +1962,36 @@ def _get_max_neighbor_distance(struct, shell):
         return max(distances)
 
     @staticmethod
-    def _get_disordered_substructure(struc_disordered):
+    def _get_disordered_substructure(struct_disordered):
         """Converts disordered structure into a substructure consisting of only disordered sites.
 
         Args:
-            struc_disordered: pymatgen disordered Structure object.
+            struct_disordered: pymatgen disordered Structure object.
 
         Returns:
             pymatgen Structure object representing a substructure of disordered sites.
         """
-        disordered_substructure = struc_disordered.copy()
+        disordered_substructure = struct_disordered.copy()
 
         idx_to_remove = [idx for idx, site in enumerate(disordered_substructure) if site.is_ordered]
         disordered_substructure.remove_sites(idx_to_remove)
 
         return disordered_substructure
 
     @staticmethod
-    def _sqs_cluster_estimate(struc_disordered, cluster_size_and_shell: dict[int, int] | None = None):
+    def _sqs_cluster_estimate(struct_disordered, cluster_size_and_shell: dict[int, int] | None = None):
         """Set up an ATAT cluster.out file for a given structure and set of constraints.
 
         Args:
-            struc_disordered: disordered pymatgen Structure object
+            struct_disordered: disordered pymatgen Structure object
             cluster_size_and_shell: dict of integers {cluster: shell}.
 
         Returns:
             dict of {cluster size: distance in angstroms} for mcsqs calculation
         """
         cluster_size_and_shell = cluster_size_and_shell or {2: 3, 3: 2, 4: 1}
 
-        disordered_substructure = SQSTransformation._get_disordered_substructure(struc_disordered)
+        disordered_substructure = SQSTransformation._get_disordered_substructure(struct_disordered)
 
         clusters = {}
         for cluster_size, shell in cluster_size_and_shell.items():

tests/apps/battery/test_analyzer.py

@@ -39,7 +39,7 @@ def test_oxide_check(self):
         with pytest.raises(ValueError, match="BatteryAnalyzer requires oxidation states assigned to structure"):
             BatteryAnalyzer(struct, "Li")
 
-    def test_capacitygrav_calculations(self):
+    def test_capacity_grav_calculations(self):
         li_fe_p_o4_cap = 169.89053  # same as fe_po4 cap
         na_fe_p_o4_cap = 154.20331
         la2_co_o4_f_cap = 175.6564
@@ -63,7 +63,7 @@ def test_capacitygrav_calculations(self):
         assert self.li3v2p3o12.get_max_capgrav(insert=False) == approx(li3_v2_p3_o12_cap_remove, abs=1e-3)
         assert self.li3v2p3o12.get_max_capgrav(remove=False) == approx(li3_v2_p3_o12_cap_insert, abs=1e-3)
 
-    def test_capacityvol_calculations(self):
+    def test_capacity_vol_calculations(self):
         li_fe_p_o4_cap = 594.17518
         na_fe_p_o4_cap = 542.86104
 
@@ -82,7 +82,9 @@ def test_capacityvol_calculations(self):
         assert self.fe_p_o4.get_max_capvol(insert=False) == 0
 
         # give the lifepo4 volume, should get lifepo4 capacity
-        assert self.fe_p_o4.get_max_capvol(volume=self.li_fe_p_o4.struc_oxid.volume) == approx(li_fe_p_o4_cap, abs=1e-3)
+        assert self.fe_p_o4.get_max_capvol(volume=self.li_fe_p_o4.struct_oxid.volume) == approx(
+            li_fe_p_o4_cap, abs=1e-3
+        )
 
     def test_ion_removal(self):
         assert self.lifemnpo4.get_removals_int_oxid() == {1, 2, 3, 4}

tests/files/.pytest-split-durations

@@ -672,8 +672,8 @@
     "tests/analysis/xas/test_spectrum.py::TestXAS::test_str": 0.002054751035757363,
     "tests/analysis/xas/test_spectrum.py::TestXAS::test_as_from_dict": 0.002419874945189804,
     "tests/analysis/xas/test_spectrum.py::TestXAS::test_validate": 0.0020956239895895123,
-    "tests/apps/battery/test_analyzer.py::TestBatteryAnalyzer::test_capacitygrav_calculations": 0.015028917114250362,
-    "tests/apps/battery/test_analyzer.py::TestBatteryAnalyzer::test_capacityvol_calculations": 0.011784416041336954,
+    "tests/apps/battery/test_analyzer.py::TestBatteryAnalyzer::test_capacity_grav_calculations": 0.015028917114250362,
+    "tests/apps/battery/test_analyzer.py::TestBatteryAnalyzer::test_capacity_vol_calculations": 0.011784416041336954,
     "tests/apps/battery/test_analyzer.py::TestBatteryAnalyzer::test_ion_removal": 0.052120123989880085,
     "tests/apps/battery/test_analyzer.py::TestBatteryAnalyzer::test_oxide_check": 0.011619041964877397,
     "tests/apps/battery/test_conversion_battery.py::TestConversionElectrode::test_composite": 0.02429362601833418,

tests/io/test_atat.py

@@ -87,28 +87,28 @@ def test_mcsqs_export(self):
 
     def test_mcsqs_cif_nacl(self):
         # CIF file from str2cif (utility distributed with atat)
-        struc_from_cif = Structure.from_file(f"{test_dir}/bestsqs_nacl.cif")
+        struct_from_cif = Structure.from_file(f"{test_dir}/bestsqs_nacl.cif")
 
         # output file directly from mcsqs
-        struc_from_out = Structure.from_file(f"{test_dir}/bestsqs_nacl.out")
+        struct_from_out = Structure.from_file(f"{test_dir}/bestsqs_nacl.out")
 
-        assert struc_from_cif.matches(struc_from_out)
+        assert struct_from_cif.matches(struct_from_out)
         assert_allclose(
-            struc_from_out.lattice.parameters,
-            struc_from_cif.lattice.parameters,
+            struct_from_out.lattice.parameters,
+            struct_from_cif.lattice.parameters,
             atol=1e-4,
         )
 
     def test_mcsqs_cif_pzt(self):
         # CIF file from str2cif (utility distributed with atat)
-        struc_from_cif = Structure.from_file(f"{test_dir}/bestsqs_pzt.cif")
+        struct_from_cif = Structure.from_file(f"{test_dir}/bestsqs_pzt.cif")
 
         # output file directly from mcsqs
-        struc_from_out = Structure.from_file(f"{test_dir}/bestsqs_pzt.out")
+        struct_from_out = Structure.from_file(f"{test_dir}/bestsqs_pzt.out")
 
-        assert struc_from_cif.matches(struc_from_out)
+        assert struct_from_cif.matches(struct_from_out)
         assert_allclose(
-            struc_from_out.lattice.parameters,
-            struc_from_cif.lattice.parameters,
+            struct_from_out.lattice.parameters,
+            struct_from_cif.lattice.parameters,
             atol=1e-4,
         )