Repository navigation
fix: quote YAML 1.1 booleans, and align task lists, gene pruning and YAML floats with raven-toolbox - #11
Open
edkerk wants to merge 8 commits into
Open
fix: quote YAML 1.1 booleans, and align task lists, gene pruning and YAML floats with raven-toolbox#11edkerk wants to merge 8 commits into
edkerk wants to merge 8 commits into
Conversation
needsQuote now quotes y, n, yes, no, on and off in any case, which YAML 1.1 loaders such as PyYAML read as booleans. This matches the quoting of write_yaml_model in raven-toolbox (CompMetEng/raven-toolbox#178). #10
Contributor
Function test results432 tests 425 ✅ 1m 57s ⏱️ Results for commit 875e45b. ♻️ This comment has been updated with latest results. |
…es header case, keeps empty CHANGED bounds as NaN
… read back exactly
edkerk
changed the title
… that is left By name, replaceMets deletes the replacement's row and keeps the renamed replaced metabolite. The ids it passed to the final contractModel call (repMetIds) were captured from the replacement only, so after the deletion none of them was found, repIdx was empty and contractModel(model,'mets',[]) checked every reaction. Duplicate reactions that do not involve the merged metabolite were merged as well. repMetIds now also holds the id of the replaced metabolite, so the restriction follows whichever of the two rows survives, and an empty list no longer reaches contractModel. Tests: unrelated duplicate reactions survive a merge by name and by id, and the new duplicate is still contracted.
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.
Closes #10.
YAML booleans
src/io/writeYAMLmodel.mneedsQuote:NO,yes,onand the other YAML 1.1 booleans (y,n,yes,no,on,offin any case) were written unquoted, so a YAML 1.1 loader such as PyYAML read nitric oxide'sname: NOasFalse. They are quoted, astrueandfalsealready were. raven-toolbox quotes the same set since fix: four writer, task and gene-pruning problems from #178 raven-toolbox#179.[Yy][Ee][Ss]) instead of(?i), which in MATLAB would also make the numeric alternatives case-insensitive and quote values such as0X1Fthat raven-toolbox leaves plain.Parity decisions from the RAVEN vs raven-toolbox review
Each item has a test. The Python side is CompMetEng/raven-toolbox#181.
writeYAMLmodel: non-integers are written with the shortest digits that read back exactly (15 to 17 significant digits), so 1/3 is0.3333333333333333and not0.333333333333333. Python writes the same.removeLowScoreGenes: when all isozymes are negative and tied, the gene listed first in the rule is kept, not the alphabetically first one. Surviving genes keep rule order. The docstring no longer says the tie is random.parseTaskList:SHOULD FAILandPRINT FLUXread0,falseandno(any case) as false.;lists are trimmed and empty items dropped.CHANGED LBorCHANGED UBis NaN, meaning the bound is left unchanged.checkTasksandfitTasksskip NaN bounds. Before, an empty bound left the bound arrays shorter than the list of changed reactions.checkTasks: warns when a task metabolite matches several model metabolites (only the first is constrained; raven-toolbox constrains all and warns too).Checks
Not run in MATLAB. The new tests (
tTasks,tINIT,tIO) need a MATLAB run before merging. Two places where a test, not the fix, could fail: theparseTaskListtest depends on howtextscanreads empty fields, and the float test setslb(1)above zero on a reaction that may be marked reversible.