-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added memote consistency tests #50
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes required, otherwise looks good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small things :) good job though!
made the suggested changes to find_disconnected_metabolites
Co-authored-by: Mirek Kratochvil <exa.exa@gmail.com>
changed to use config defaults and isapprox()
use COBREXA constants
added the internal helper function `_compare_flux_bounds` which will be used in `unbounded_flux_in_default_medium`
changed to use the helper function `_compare_flux_bounds`
/format |
triggered by @Fl-Sch on PR LCSB-BioCore#50
✔️ Auto-formatting triggered by this comment succeeded, commited as 3ccb851 |
/format |
triggered by @exaexa on PR LCSB-BioCore#50
✔️ Auto-formatting triggered by this comment succeeded, commited as c9940eb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me except the tests should pass (I guess committing the suggestions from the comments should fix it)
Codecov ReportBase: 95.27% // Head: 95.45% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #50 +/- ##
==========================================
+ Coverage 95.27% 95.45% +0.17%
==========================================
Files 16 16
Lines 572 594 +22
==========================================
+ Hits 545 567 +22
Misses 27 27
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok nvm, managed to fix :]
We added the metabolite connectivity and unbounded flux in default medium tests. We are not sure about the structural elements, like if we correctly added to the consistency config.