Merge branch 'DAOTHER-10246' into develop

Author: Ezra Peisach

scripts/dump_db_to_cif.py

@@ -32,7 +32,6 @@
 
 try:
     import gemmi
-    from gemmi.cif import Style, WriteOptions
 except ImportError as e:
     sys.stderr.write("Error: gemmi library is required but not installed.\n")
     sys.stderr.write("Please install it with: pip install gemmi\n")
@@ -336,9 +335,10 @@ def format_cif_loop_value(value: Any, allow_multiline: bool = False) -> str:
         pass
 
     # For multiline text in loops, use semicolon format to preserve newlines
+    # Need newline after close
     if allow_multiline and "\n" in str_value:
         escaped = escape_non_ascii(str_value, preserve_newlines=True)
-        return f"\n;{escaped}\n;"
+        return f"\n;{escaped}\n;\n"
 
     # For single-line values, escape and flatten
     escaped = escape_non_ascii(str_value, preserve_newlines=False)
@@ -633,12 +633,7 @@ def _export_messages_to_cif(self, messages: List[MessageInfo], file_path: str,
             # Ensure output directory exists
             os.makedirs(os.path.dirname(file_path), exist_ok=True)
 
-            # Write CIF file - use as_string() to avoid alignment issues with write_file()
-            # gemmi's write_file() with align_loops causes spacing issues that break parsing
-            opts = WriteOptions(Style.Pdbx)
-            cif_content = doc.as_string(opts)
-            with open(file_path, 'w') as f:
-                f.write(cif_content)
+            doc.write_file(file_path)
 
             log_event("file_exported", deposition_id=deposition_id, file_path=file_path,
                      content_type=content_type, messages=len(messages))
@@ -659,7 +654,16 @@ def _export_messages_to_cif(self, messages: List[MessageInfo], file_path: str,
             return False
 
     def _add_message_info_category(self, block: gemmi.cif.Block, messages: List[MessageInfo]):
-        """Add _pdbx_deposition_message_info category to CIF block"""
+        """Add _pdbx_deposition_message_info category to CIF block
+        
+        Note: This implementation manually distinguishes between pairs (single item) and 
+        loops (multiple items). A better approach for future refactoring would be to use 
+        gemmi's block.find_or_add() which handles this automatically:
+            table = block.find_or_add(prefix, tags)
+            table.append_row(values)
+        This would let gemmi decide whether to write as pairs or loops, and handle quoting
+        automatically. See: https://gemmi.readthedocs.io/en/latest/cif.html#pairs-and-loops
+        """
         # Define the columns we want to include
         columns = [
             "ordinal_id",
@@ -698,7 +702,11 @@ def _add_message_info_category(self, block: gemmi.cif.Block, messages: List[Mess
                 block.set_pair(f"_pdbx_deposition_message_info.{col}", formatted_value)
 
     def _add_file_reference_category(self, block: gemmi.cif.Block, file_refs: List[MessageFileReference]):
-        """Add _pdbx_deposition_message_file_reference category to CIF block"""
+        """Add _pdbx_deposition_message_file_reference category to CIF block
+        
+        Note: See _add_message_info_category() for recommended refactoring approach using
+        gemmi's find_or_add() to avoid manual pair/loop branching.
+        """
         columns = [
             "ordinal_id",
             "message_id",
@@ -728,7 +736,11 @@ def _add_file_reference_category(self, block: gemmi.cif.Block, file_refs: List[M
                 block.set_pair(f"_pdbx_deposition_message_file_reference.{col}", formatted_value)
 
     def _add_status_category(self, block: gemmi.cif.Block, statuses: List[MessageStatus]):
-        """Add _pdbx_deposition_message_status category to CIF block"""
+        """Add _pdbx_deposition_message_status category to CIF block
+        
+        Note: See _add_message_info_category() for recommended refactoring approach using
+        gemmi's find_or_add() to avoid manual pair/loop branching.
+        """
         columns = [
             "message_id",
             "deposition_data_set_id",