writeYAMLmodel: drop empty genes/gene_reaction_rule fields - #715
Conversation
isfield(model,'genes') alone doesn't distinguish "no genes field" from "genes field present but empty" -- the latter still wrote a bare "- genes:" line with nothing after it, which YAML parses as genes: null rather than an empty list. read_yaml_model then crashes iterating over that None. Guard on ~isempty(model.genes) too, so an empty gene list is omitted the same way an absent genes field already is.
Function test results303 tests 276 ✅ 52s ⏱️ Results for commit 92c5a44. ♻️ This comment has been updated with latest results. |
genes is one of cobra's required top-level model keys -- model_to_dict always includes it, empty or not -- so raven_toolbox.io.write_yaml_model already emits "- genes: []" for a gene-less model. Omitting the section entirely, as the previous commit did, meant the two writers no longer agreed on this line. Match cobra's own convention instead, and teach readYAMLmodel.m to recognise the flow-style empty list.
|
Update: the first commit omitted the `genes:` section entirely for a gene-less model. That doesn't actually match raven-toolbox's output — `genes` is one of cobra's required top-level model keys (`model_to_dict` always includes it), so `raven_toolbox.io.write_yaml_model` already writes `- genes: []` for a gene-less model via cobra's own `model_to_dict`. Second commit switches to `- genes: []` instead, matching that exactly, and teaches `readYAMLmodel.m` to recognise the flow-style empty list. Re-verified byte-parity against raven-toolbox's actual output for the same gene-less model, and re-ran `tIO.m` (14/14 passed). |
gene_reaction_rule is one of cobra's required reaction attributes (model_to_dict always writes it), but nothing on either read side needs the key present: cobra's own Reaction defaults it to '', and raven_toolbox.io.read_yaml_model never indexes it directly, so an absent key round-trips identically to an explicit empty one. Write it like every other optional field instead (txt, not txtReq) --- the txtReq case is now unused, so it's removed too. A deliberate divergence from cobra's own convention, not an oversight; matching raven_toolbox.io.write_yaml_model (companion change there) is what matters here.
edkerk
changed the title
1 participant
Summary
Two related empty-field write fixes, both aimed at keeping
writeYAMLmodelandraven_toolbox.io.write_yaml_modelbyte-identical (companion PR: SysBioChalmers/raven-toolbox#112):isfield(model,'genes')alone didn't distinguish "no genes field" from "genes field present but empty" — the latter wrote a bare- genes:line, which YAML parses asgenes: null, crashingread_yaml_model.genesis one of cobra's required top-level model keys (model_to_dictalways includes it, empty or not), so raven-toolbox already writes- genes: []for this case. Matched that exactly rather than omitting the section, and taughtreadYAMLmodel.mto recognise the flow-style empty list.gene_reaction_rule: also one of cobra's required reaction attributes (always written bymodel_to_dict), but nothing on either read side actually needs the key present — cobra's ownReactiondefaults it to'', andread_yaml_modelnever indexes it directly. Dropped it the same way every other optional field already is. A deliberate divergence from cobra's own writer convention, matching raven-toolbox's companion change.Resolves SysBioChalmers/raven-gecko-parity#27.
Test plan
smallYeast, wrote it —- genes: [], round-trips through bothreadYAMLmodelandraven_toolbox.io.read_yaml_modelcleanlygenes:) against the unfixed reader, confirmed it matches the issue exactly, confirmed the fix resolves itsmallYeasthas 14/53 reactions with no GPR — wrote through bothwriteYAMLmodelandwrite_yaml_model, diffed the two outputs: byte-identical (SHA256 match)testing/function_tests/tIO.m— 14 passed, 0 failed (1 pre-existing unrelated skip)