feat: generalize kcat merging to N sources via mergeKcats - #457
Merged
Conversation
Add mergeKcats: merges any number of kcatLists (each of which may mix
several sources itself, e.g. an OpenKineticsPredictor result carrying
BRENDA/Sabio-RK/CataPro rows in its own per-row kcatSource field) and
resolves them per reaction by a caller-supplied sourcePriority. Three
reserved tier tokens select from a database-sourced row's own quality:
- database_exact: an exact experimental-DB hit (no wildcard/origin
metadata at all -- i.e. a direct OKP value, not a fuzzy match).
- database_top: a fuzzy BRENDA match, wildcardLvl==0 & origin<=top.
- database_bottom: weaker fuzzy matches within the wildcard/origin gate.
Any other sourcePriority token is matched against a row's own source
label (folded to lowercase snake_case, e.g. 'dlkcat'); sources not
listed in sourcePriority are dropped, with a warning naming them.
mergeDLKcatAndFuzzyKcats becomes a deprecated, warning-emitting thin
wrapper delegating to mergeKcats with the legacy three-tier order
('database_top', 'dlkcat', 'database_bottom') and its own established
parseGECKOargs-style optional arguments unchanged. Verified against
the existing testKcats_tc0011 fixture (unmodified): identical merged
output to the pre-rewrite algorithm, confirming the tier-based
reimplementation preserves the legacy function's exact row selection.
Adds testMergeKcatsGeneralizesToNSourcesWithExactTier_tc0041 (the new
database_exact tier and the dropped-source warning, using a
fetchOpenKineticsPredictor-shaped fixture) and
testMergeDLKcatAndFuzzyKcatsDelegatesToMergeKcats_tc0042 (the wrapper's
own three-tier order, independent of testKcats_tc0011's larger
end-to-end fixture).
Full local suite: 42/42 passed.
Unit test results43 tests 43 ✅ 11s ⏱️ Results for commit 07d32fa. ♻️ This comment has been updated with latest results. |
…erge-kcats # Conflicts: # test/unit_tests/geckoCoreFunctionTests.m
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
Adds
mergeKcats: merges any number of kcatLists (each of which may mix several sources itself, e.g. an OpenKineticsPredictor result carrying BRENDA/Sabio-RK/CataPro rows in its own per-rowkcatSourcefield) and resolves them per reaction by a caller-suppliedsourcePriority. Three reserved tier tokens select from a database-sourced row's own quality:database_exact— an exact experimental-DB hit (no wildcard/origin metadata at all — i.e. a direct OKP value, not a fuzzy match).database_top— a fuzzy BRENDA match,wildcardLvl==0 & origin<=top.database_bottom— weaker fuzzy matches within the wildcard/origin gate.Any other
sourcePrioritytoken is matched against a row's own source label (folded to lowercase snake_case, e.g.'dlkcat'); sources not listed insourcePriorityare dropped, with a warning naming them.mergeDLKcatAndFuzzyKcatsbecomes a deprecated, warning-emitting thin wrapper delegating tomergeKcatswith the legacy three-tier order ('database_top','dlkcat','database_bottom') and its own establishedparseGECKOargs-style optional arguments unchanged.Test plan
testKcats_tc0011fixture (unmodified): identical merged output to the pre-rewrite algorithm, confirming the tier-based reimplementation preserves the legacy function's exact row selection.testMergeKcatsGeneralizesToNSourcesWithExactTier_tc0041(the newdatabase_exacttier and the dropped-source warning, using afetchOpenKineticsPredictor-shaped fixture).testMergeDLKcatAndFuzzyKcatsDelegatesToMergeKcats_tc0042(the wrapper's own three-tier order, independent oftestKcats_tc0011's larger end-to-end fixture).