Robustness and polish fixes#25
Merged
Merged
Conversation
Tier 4 of the review: small, targeted hardening, no behaviour change on valid input. - gapfilling/fill.py: clamp the connectivity gap-fill big-M to the largest finite bound magnitude, so a template reaction with an infinite bound no longer puts an infinite coefficient into the MILP (which broke the solver). - reconstruction/kegg/download.py: a malformed or unreadable .netrc now raises a ValueError explaining how to fix it, instead of a raw NetrcParseError/OSError. - io/excel.py: always write the metabolite formula to the METS COMPOSITION column; it was dropped whenever an InChI was present. - visualization: the empty stub package raises a clear NotImplementedError (with a roadmap pointer) on attribute access, via a PEP 562 module __getattr__. A regression test per fix.
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
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.
Tier 4 of the raven-python review: small, targeted robustness / polish fixes — no
behaviour change on valid inputs.
Fixes
gapfilling/fill.py— the connectivity gap-fill MILP used each templatereaction's bound directly as the big-M (
flux <= ub * y). A template reaction withan infinite bound therefore put an infinite coefficient into the problem, breaking
the solver. The big-M is now clamped to the largest finite bound magnitude in the
model (>= any flux the reaction can carry).
reconstruction/kegg/download.py— a malformed or unreadable~/.netrcraised araw
NetrcParseError/OSErrorout of_resolve_auth. It now raises aValueErrorexplaining how to fix the file, matching the other credential errors there.
io/excel.py— the METS-sheet exporter dropped a metabolite'sformula(theCOMPOSITION column) whenever an InChI was present. It now always writes the formula,
so nothing is silently lost in the RAVEN Excel export.
visualization/— the empty stub package now raises a clearNotImplementedError(with a roadmap pointer) on any attribute access, via a PEP 562 module
__getattr__,instead of a bare
AttributeError.Tests
One regression test per fix: gap-fill with an inf-bounded template, malformed-
.netrcerror, formula-kept-when-InChI-present (the existing test that asserted the drop is
updated accordingly), and the visualization-stub error.
Deliberately not changed
The plan also suggested pinning a
pytest-covfloor in thedevextra. That edits thesame
dev = [...]block as #22 (which addsmypy) for marginal value, so it is deferredto avoid a needless merge conflict.
Verification
pytest(621 passed) andruff checkare green.mypyflags only pre-existingerrors in
fill.py/download.py/excel.py(untyped optlang /notesaccess),all within #22's type-fix scope — the lines added here introduce none (verified against
the develop baseline by diffing error counts). raven-python's CI gains its
mypyjobwith #22.