fix: four EC-code and kcat data-quality bugs - #446
Merged
Conversation
- fuzzyKcatMatching.m: the wildcard-escalation loop had no termination guard, so an EC class with zero BRENDA coverage at every wildcard level crashed instead of reporting no match (#442). - getECfromGEM.m: the EC validation regex used \w instead of \d+ per component, so malformed codes containing letters or underscores (e.g. "1.1.1.n12", "1_2_3_4") passed validation instead of being rejected (#443). - findECInDB.m: the intersection() helper didn't dedupe its output, so a specific EC code and its own wildcard parent could both resolve to the same code and get appended twice (#444). - getStandardKcat.m: kcatSubSystemIdx is a one-hot vector, so all(kcatSubSystemIdx) only ever succeeds for a model with exactly one subSystem; any() is the correct test. With more than one subSystem, every reaction silently fell back to the model-wide standardKcat instead of its own subSystem-specific value (#445). Each is covered by a new regression test (tc0031-tc0034), verified to fail against the pre-fix code and pass against the fix. Closes #442, #443, #444, #445.
Unit test results34 tests 34 ✅ 10s ⏱️ Results for commit e7ec543. |
This was referenced Aug 28, 2026
Closed
Closed
edkerk
added a commit
to SysBioChalmers/raven-gecko-parity
that referenced
this pull request
Aug 28, 2026
findECInDB, fuzzyKcatMatching, getECfromGEM, and getStandardKcat each had a real, confirmed-live MATLAB bug, originally diagnosed and fixed on the long-abandoned feat/geckopy-compat branch (84 ahead of develop4 but also 99 behind it, so not directly mergeable). Filed as SysBioChalmers/GECKO#442-445, fixed and re-verified against current develop4 in SysBioChalmers/GECKO#446 (open). Three of the four already had their corrected behaviour documented or implemented on the Python side (geckopy's own MATLAB-COMPAT notes, or just correct-by-construction code), used here to confirm each fix brings MATLAB in line rather than just silencing an error. The getECfromGEM fix also closes an already-documented ledger divergence (malformed EC codes like "1.1.1.n12" on yeast-GEM).
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
Four small, independent bug fixes to EC-code handling and kcat gathering, each with its own tracking issue and regression test:
-.-.-.-) crashed instead of reporting no match.\winstead of\d+per component, so malformed codes containing letters or underscores (e.g.1.1.1.n12,1_2_3_4) passed validation instead of being rejected. This also closes an already-documented divergence inraven-gecko-parity's ledger, wheregeckopy.fill_eccodes_from_gemcorrectly rejects these andgetECfromGEM.mdidn't.intersection()helper didn't dedupe its output, so a specific EC code and its own wildcard parent (e.g.1.1.1.1and1.1.1.-) could both resolve to the same code and get appended twice.kcatSubSystemIdxis a one-hot vector, soall(kcatSubSystemIdx)only ever succeeds for a model with exactly one subSystem in total;any()is the correct test. With more than one subSystem, every reaction silently fell back to the model-widestandardKcatinstead of its own subSystem-specific value.All four were originally diagnosed and fixed on the long-abandoned
feat/geckopy-compatbranch (84 commits ahead ofdevelop4, but also 99 behind it — not mergeable as-is), while building outraven-gecko-parity's MATLAB/Python behavioural parity tests. Re-verified against currentdevelop4and re-implemented here rather than merging that branch.In three of the four cases, geckopy's own port already documents (or simply implements) the corrected behavior, which was used to confirm the fix matches the intended cross-language behaviour rather than just silencing an error.
Test plan
testGetECfromGEMRejectsMalformedComponents_tc0031,testFindECInDBIntersectionDedupesWildcardPair_tc0032,testFuzzyKcatMatchingWildcardExhaustionDoesNotCrash_tc0033,testGetStandardKcatUsesSubsystemKcatWheneverAnyMatches_tc0034togeckoCoreFunctionTests.m.Closes #442, #443, #444, #445.