Fix load_delta_g_csv stamping the ΔG missing-value sentinel as a real value - #73
Merged
Conversation
…alue
The ModelSEED-derived side-car tables encode "no valid dG" as the magic
value 10000000. load_delta_g_csv -- written for exactly these files, down
to its Var1/Var2 column defaults -- had no notion of it and stamped it
verbatim, so 777 of yeast-GEM's 4102 reactions (19.5%) carried a
physically impossible 1e7 kJ/mol presented as a measurement. Anything
reading notes["deltaG"] got garbage for a fifth of the model.
yeast-GEM's own checkrxnDirection.m gates on the same value:
if ~isequal(seed_rxnInfo{rxnIdx4(i),16},'10000000.0')
%check if database contains valid deltaG value
Treat the sentinel as missing, exactly as NaN already was, recognising it
whichever dtype the MATLAB/pandas round-trip produces (10000000,
10000000.0, "10000000.0"). The new keyword-only missing_value (default
SEED_DELTA_G_MISSING) tunes or disables it.
Real dG coverage of yeast-GEM is 78.2% (3207/4102), not the 97.1% the
loader previously implied.
The facet set is localization + equation + gene_association, and no further facet is planned. Drop the planned fourth facet from the study doc and from confidence.py's module docstring, which both still promised one. Replace the phasing section, whose P1/P2 entries only restated what had already shipped, with what is actually left: * Wire the facets together -- an annotate_confidence() umbrella, and let curation_priority read the record so a mark_curated reaction stops resurfacing in the review queue. That is the change that closes the score -> review -> curate loop. * Validate beyond one model. Every number in the doc is yeast-GEM's, and three bands never fire there. * Standards alignment for the paper (Thiele-Palsson / ECO); the SBO half is already done.
…inel The 10000000 "no valid ΔG" sentinel handled by load_delta_g_csv was described as ModelSEED's and the constant named SEED_DELTA_G_MISSING, but the ΔG side-car tables (e.g. yeast-GEM's model_rxnDeltaG.csv) are not ModelSEED-derived — the shared sentinel value does not establish that provenance. Describe it neutrally as the tables' own missing-value marker (still evidenced by yeast-GEM's checkrxnDirection.m gating on 10000000.0) and rename the constant to DELTA_G_MISSING. No behaviour change.
This was referenced Jul 18, 2026
# Conflicts: # CHANGELOG.md
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
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.
load_delta_g_csvstamped the ΔG side-car tables' "missing" sentinel (10000000, e.g. in yeast-GEM'sdata/databases/model_rxnDeltaG.csv) verbatim, so a reaction with no valid ΔG carried a physically impossible 10⁷ kJ/mol as if it were a measurement — 777 of yeast-GEM's 4102 reactions.The sentinel is now treated as missing, exactly as
NaNalready was, recognised whichever dtype the CSV round-trip produces (10000000,10000000.0,"10000000.0"). New keyword-onlymissing_value(defaultDELTA_G_MISSING, exported) tunes or disables it. Three regression tests, in the style of the existingtest_load_skips_nan_rows.