readYAMLmodel: fix quoted list items losing their quotes - #712
Merged
Conversation
The list-marker strip ("- " at the front of a sequence entry) ran
after quote detection/stripping, not before. A quoted scalar (key:
"value") starts with the quote character right where the
detection/stripping regexes anchor, so that path worked --- but a
quoted list item (e.g. a pubmed id under an annotation block,
- "10714900"
) still had its leading "- " attached at that point, so the anchored
^".*"$ / ^'.*'$ checks never matched, and the quotes were never
stripped or unescaped.
Reordering the two steps (strip the list marker first) is the whole
fix: once the marker is gone, a quoted list item's value has the exact
same shape as a quoted scalar, and the existing detection/stripping/
unescaping logic already handles it correctly.
Silent, not loud: exportForGit's SBML writer embeds MIRIAM values
inside double-quote-delimited XML attributes, so a still-quoted pubmed
id breaks that attribute and the whole <annotation> block for that
reaction is dropped rather than erroring.
Verified: a pubmed list item now reads back as the bare id, not the
quoted string; a single-quoted list item with a doubled apostrophe
still unescapes correctly (the reorder doesn't regress that path);
tSyntax/tIO both pass (15/15).
Function test results303 tests 276 ✅ 1m 7s ⏱️ Results for commit 9a7949f. |
2 tasks
edkerk
added a commit
that referenced
this pull request
Aug 28, 2026
…lt" (#713) Reverts the quote-character choice from #710 back to single quotes. writeYAMLmodel emits single quotes when a scalar needs quoting again, exactly as before #710 (verified: byte-identical output to the pre-#710 writer on tutorial/smallYeast.yml, same SHA256). readYAMLmodel's unescaping keeps handling both quote styles (it already did, to read files written either way) and #712's list-item quote-marker-stripping fix is untouched --- that bug affected quoted list items under either quote convention and isn't specific to which character is used, so there's no reason to revert it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Regression from #710 (single→double quote switch), found via real-world testing against yeast-GEM.
The list-marker strip (
"- "at the front of a sequence entry) ran after quote detection/stripping, not before. A quoted scalar (key: "value") starts with the quote character right where the detection/stripping regexes anchor, so that path worked — but a quoted list item (e.g. a pubmed id under an annotation block,- "10714900") still had its leading"- "attached at that point, so the anchored^".*"$/^'.*'$checks never matched, and the quotes were never stripped or unescaped.Silent, not loud:
exportForGit's SBML writer embeds MIRIAM values inside double-quote-delimited XML attributes, so a still-quoted pubmed id breaks that attribute and the whole<annotation>block for that reaction is dropped rather than erroring. Reported as affecting 296/296pubmed-bearing reactions in yeast-GEM.Fix is a pure reorder — no regex changes: strip the list marker before quote detection/stripping runs, so a quoted list item's value has the same shape as a quoted scalar by the time the existing (correct) logic sees it.
Test plan
pubmed:list item (- "10714900") now reads back as the bare id10714900, not the quoted string- 'yeast''s value') still unescapes correctly — confirms the reorder doesn't regress that pathtSyntax,tIO— 15 passed, 0 failed, 1 filtered (unrelated missing dependency)readYAMLmodel→exportForGit) not run here — the synthetic repro above isolates the exact mechanism, but this is worth someone re-confirming against the real corpus before/after merge