Added fast gapfilling flag. All tests pass with fast_gapfill=True #1450
Conversation
…ck needs to happen instead of nrxns checks
|
Hey folks, apologies for the slow turnaround. I finally got around to fixing the linting errors. Please rerun the workflow and it should hopefully pass all the tests, now. |
|
Hmm... the unit tests failed because the Biomodels.net website was down. Reported this issue #1455 and suggested a fix. |
| The threshold at which a value is considered non-zero (aka | ||
| integrality threshold). If gapfilled models fail to validate, | ||
| you may want to lower this value (default 1E-6). | ||
| fast_gapfill : bool, optional |
There was a problem hiding this comment.
Because this is already a gapfill function I think having it in the argument name is redundant. Just calling it "fast" should be enough. Or call it "method" and make it a string to allow for other algorithms in the future.
| iterations pathways including 10 steps will eventually be reported | ||
| even if the shortest pathway is a single reaction (default 1). | ||
| fast_gapfill : bool, optional |
| The total number of reactions in the model having ID `model_id`. | ||
| """ | ||
| if os.getenv("CI") == "true": |
There was a problem hiding this comment.
Better remove this here and let's solve this in a separate PR which we can then rebase on. Right now this is also missing the os import which is making the CI fail. Thx
|
Looking great. I will start a separate PR for the Biomodels tests issue we might need to merge beforehand. |
2 participants
fast_gapfilltocobra.flux_analysis.gapfilling.gapfill()that, whenTrue, changes the indicator variable types to becontinuousinstead ofbinary, enabling fast gapfillingtest_fast_gapfillingtest intests/test_flux_analysis/test_gapfilling.pyand it passes