Skip to content

to_mmcif: quote empty pdbx_strand_id so _struct_ref_seq stays readable - #433

Merged
keitaroyam merged 1 commit into
project-gemmi:masterfrom
martinemnoble1:fix-empty-pdbx-strand-id
Jun 18, 2026
Merged

to_mmcif: quote empty pdbx_strand_id so _struct_ref_seq stays readable#433
keitaroyam merged 1 commit into
project-gemmi:masterfrom
martinemnoble1:fix-empty-pdbx-strand-id

Conversation

@martinemnoble1

Copy link
Copy Markdown
Contributor

Problem

For a structure with a blank chain name (e.g. a PDB file with no chain ID) and a DBREF, make_mmcif_document() produces an mmCIF file that gemmi itself cannot read back:

import gemmi
st = gemmi.read_structure('1pfe.cif.gz')
for ch in st[0]:
    if ch.name == 'B':
        ch.name = ''          # blank chain name + existing DBREF
doc = st.make_mmcif_document()
gemmi.cif.read_string(doc.as_string())
# ValueError: Wrong number of values in loop _struct_ref_seq.*

The empty _struct_ref_seq.pdbx_strand_id is written as a zero-width token, so the data row ends up with one value fewer than the loop has tags. The loop is ragged, and the whole file becomes unreadable — not just by gemmi's strict reader, but by other consumers too (this surfaced as an MMDB/clipper Message_fatal abort in a downstream pipeline reading gemmi-written mmCIF).

It reproduces on current master and on the 0.7.5 release. Real-world trigger: legacy single-chain PDB entries with a blank chain ID that carry DBREF records (e.g. PDB 2ACY).

Cause

In the _struct_ref_seq row builder (src/to_mmcif.cpp), every field is passed through a sanitising helper (string_or_qmark, string_or_dot, .str(), qchain) — except pdbx_strand_id, which was added raw:

seq_loop.add_row({std::to_string(++counter2),
                  std::to_string(counter),
                  strand_id->second,  // pdbx_strand_id  <-- raw; empty -> zero-width token
                  id,
                  ...

When strand_id->second is empty, nothing is emitted for that column.

Fix

Wrap it in qchain() — exactly what the adjacent _struct_ref row already does for qchain(ent.name), and what qchain's own comment describes (chain names that may be empty, written as ''). The empty value now serialises as '' (a valid quoted empty string), the loop is well-formed, and it round-trips.

qchain(strand_id->second),  // pdbx_strand_id

One-line change. Added a regression test (tests/test_mol.py::test_empty_strand_id_struct_ref_seq) that blanks a chain name on 1pfe and asserts the written _struct_ref_seq re-reads cleanly; it fails before the change (Wrong number of values in loop _struct_ref_seq) and passes after. Full test_mol.py/test_cif.py pass with no regressions.

When a structure has a blank chain name (e.g. a PDB file with no chain ID)
and a DBREF, the _struct_ref_seq.pdbx_strand_id value is empty. It was added
to the loop raw, so the writer emitted it as a zero-width token: the data row
then had one value fewer than the loop has tags, making the whole mmCIF file
unreadable (gemmi's own reader rejects it with "Wrong number of values in
loop _struct_ref_seq", and strict third-party readers such as MMDB/clipper
abort on it).

Every sibling field in this row is already passed through a helper
(string_or_qmark, string_or_dot, .str(), qchain); pdbx_strand_id was the only
raw one. Wrap it in qchain(), matching the adjacent _struct_ref row's
qchain(ent.name) and qchain's documented purpose (chain names, empty -> '').
The empty value now serialises as '' and the loop round-trips.

Added a regression test that blanks a chain name on 1pfe and checks the
written _struct_ref_seq re-reads cleanly.
@keitaroyam
keitaroyam merged commit 8fd2846 into project-gemmi:master Jun 18, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants