Fix blank line bug in io.res.ResWriter (#3671)

* Fix end of file newline for res file * Add test for blank line bug * Add fix for blank line bug * refactor pymatgen/io/res.py to use list concat --------- Co-authored-by: Janosh Riebesell <[email protected]>
materialsproject · Mar 5, 2024 · 26cf207 · 26cf207
1 parent b96d9e1
commit 26cf207
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 10 deletions.
diff --git a/pymatgen/io/res.py b/pymatgen/io/res.py
@@ -91,7 +91,7 @@ class ResSFAC:
     def __str__(self) -> str:
         species = " ".join(f"{specie:<2s}" for specie in self.species)
         ions = "\n".join(map(str, self.ions))
-        return f"SFAC {species}\n{ions}\nEND"
+        return f"SFAC {species}\n{ions}\nEND\n"
 
 
 @dataclass(frozen=True)
@@ -104,15 +104,13 @@ class Res:
     SFAC: ResSFAC
 
     def __str__(self) -> str:
-        return "\n".join(
-            [
-                "TITL" if self.TITL is None else str(self.TITL),
-                "\n".join(f"REM {rem}" for rem in self.REMS),
-                str(self.CELL),
-                "LATT -1",
-                str(self.SFAC),
-            ]
-        )
+        lines = ["TITL" if self.TITL is None else str(self.TITL)]
+
+        lines += (f"REM {rem}" for rem in self.REMS)
+
+        lines += (str(self.CELL), "LATT -1", str(self.SFAC))
+
+        return "\n".join(lines)
 
 
 class ResParseError(ParseError):

diff --git a/tests/io/test_res.py b/tests/io/test_res.py
@@ -68,6 +68,18 @@ def test_misc(self, provider: AirssProvider):
         assert airss_v[0] == "0.9.1"
         assert airss_v[1].year == 2018
 
+    def test_end_of_file_newline(self, provider: AirssProvider):
+        """https://github.com/materialsproject/pymatgen/issues/3636"""
+        string = ResWriter(provider.entry).string
+        assert string.endswith("\n")
+
+    def test_empty_rems_no_blank_line(self, provider: AirssProvider):
+        """https://github.com/materialsproject/pymatgen/issues/3636"""
+        entry = provider.entry
+        del entry.data["rems"]
+        string = ResWriter(entry).string
+        assert all(string.splitlines())
+
     def test_entry(self, provider: AirssProvider):
         entry1 = provider.entry
         assert entry1.energy == provider.energy