Repository navigation
Fixes from the RAVEN 3 Guide review - #175
Merged
Merged
Conversation
read_yaml_model wrapped every annotation value in a list, so annotation["sbo"] came back as ['SBO:...'] while add_sbo_terms and cobra's SBML reader give a bare string. sbo is single-valued and is now kept as a string, also when the file spells it as a one-item list.
batch_curate kept metNotes, rxnNotes, rxnReferences and rxnConfidenceScores under their table names in .notes, and the InChI in annotation["inchi"]. read_yaml_model and the YAML/MAT writers use note, references, confidence_score and inchis, so write_yaml_model emitted a nested mapping that MATLAB read back as "metNotes: ...", and a model read from YAML ended up with two different InChIs. The values now go under those keys, the confidence score as a number, and an InChI already held as an annotation is updated too.
…b the change With balance_out, scale_biomass rescaled the balancing component so the total is 1 g/gDW without checking that its new fraction stays positive. A negative factor turned the pseudoreaction's substrates into products and the model was changed without an error; a balancing component with fraction 0 was skipped silently. Both now raise before the model is changed, and the docstring says the total becomes 1 g/gDW rather than staying there.
…re growth The indicator coupling uses big-M 1000, so with the solver's default integrality tolerance (about 1e-5) a "closed" candidate could carry about 0.01 flux, enough for the default 10 % growth floor. On smallYeast this returned "optimal" with nothing added for the RPI, DAR, CIT, ACO, IDP1, CO2TRANS and GROWTH removals on GLPK. The MILP now runs with an integrality tolerance of 1e-9, as RAVEN's optimizeProb does, and the rounded selection is checked on the filled model: when it does not reach min_growth the result has exit_status "not_restored".
…EN parity as implemented The docstring credited the parity to replacing RAVEN's Monte-Carlo background, which RAVEN no longer uses, said RAVEN sorts by p-value and reports both tails, and cited an IMPROVEMENTS entry that does not exist. It now states the population standard deviation, the one-sided p-value and the z-score sort, and the constant factor a RAVEN version with the sample standard deviation differs by.
…date The module docstring presented the module as random-objective sampling only, said RAVEN has no seed and hard-codes two objectives, and cited an IMPROVEMENTS entry that does not exist. It now describes the method dispatch and the actual differences from RAVEN's random-objective method. The convergence study link points to CompMetEng/raven-docs.
The docstrings said it removes reactions that cannot carry flux in any steady state and never one that merely lacks an open boundary. Only model.exchanges are opened; sinks, demands and internal bounds keep their values.
…fault RAVEN's checkTasks leaves the model's exchanges open only on a model without boundary metabolites; prepINITModel calls closeModel first, which corresponds to close_boundaries=True.
connect_blocked_reactions and the remove_no_flux_reactions summary described blocked reactions without saying they are found under the model's current (or exchange-opened) bounds.
…_report raises find_leak_metabolite uses the model's boundary reactions as they are, so the caller has to close them, and an infeasible search does not show the model is leak-free. gap_report raises Infeasible when the model, or its closed copy for the mass-balancing section, is infeasible.
…TING docstring convention The README linked the MkDocs URL of the RAVEN 3 comparison page, which gives 404, and said every example on raven-docs is executed on each commit. CONTRIBUTING said all docstrings are NumPy style because the site's generator expects it; most are prose and the generator does not parse sections. raven-docs links point to CompMetEng.
…didates in fill_gaps_kumar_milp The method is from BMC Bioinformatics 8:212. The module docstring now says that every irreversible draft reaction, exchanges and maintenance drains included, is a reversal candidate.
…uxes gap_report raised a bare cobra Infeasible partway through when the model was infeasible under its own bounds or once its boundary was closed. It now raises a ValueError up front naming the reactions with a forced flux and asking for ATP maintenance and similar bounds to be set to zero.
Draft reactions keep their bounds; the MILP minimises the number of added template reactions. reversed_reactions and the weights argument are removed.
Without directionality reversal the function is no longer Kumar's GapFill; the name now matches RAVEN's gapFillMILP. KumarGapFillResult becomes MILPGapFillResult and the module gapfilling.kumar_milp becomes gapfilling.milp.
edkerk
force-pushed
the
fix/guide-review
branch
from
October 17, 2026 20:15
05fe538 to
cd65bee
Compare
edkerk
added a commit
to CompMetEng/raven-docs
that referenced
this pull request
Oct 22, 2026
Addresses the RAVEN 3 Guide review (comments G01–G30). The RAVEN and raven-toolbox submodules point at their `develop` heads, which include CompMetEng/RAVEN#7 and CompMetEng/raven-toolbox#175; the documented outputs come from that code. **Example checks** - `scripts/run_examples.py` compares each example with the plain ```` ```text ```` block below it, and `--update` writes that form. The examples workflow also runs on RAVEN changes. - The MATLAB MILP examples run on GLPK, showing RAVEN's warning that recommends Gurobi; the Python job needs no Gurobi licence. **Guide** - Every section the review flagged is rewritten to match what the code does: FBA, FVA, sampling, MOMA (quadratic, on a viable knockout), media, solvers, quality control, gap filling, tasks, curation, biomass, I/O, model structure, homology, KEGG, comparing models and engineering. - Gap filling: `gapFillMILP` and `fill_gaps_milp` only add reactions; the infeasible-model error of `gapReport` and `gap_report` is described. - Every example data file is listed, with how to run a page; the README and build notes describe the Sphinx build. **Reference** - On the guide pages, function names link to their API entry: RAVEN and raven-toolbox names to the generated reference, cobrapy names to cobrapy's documentation through intersphinx. The "Functions on this page" tables link every entry, prose links the first mention per section, and cobrapy badges point at the named function. - The API reference renders numpydoc help as MyST. - The tutorials include their scripts from RAVEN's `tutorials/` folder. - Pages describe RAVEN 3 and its `develop` branch; the RAVEN 2 → 3 pages compare RAVEN 2.11.3 with RAVEN 3. **Build** - The MkDocs files are removed; the Sphinx build uses `requirements.txt` and `scripts/gen_api_pages.py`, and `scripts/check_names.py` runs on its own.
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.
Fixes found while reviewing the RAVEN 3 Guide.
Gap filling
fill_gaps_kumar_milpis renamedfill_gaps_milp(MILPGapFillResult, modulegapfilling.milp), matching RAVEN'sgapFillMILP. It only adds template reactions, choosing the fewest that reach the growth floor; draft reactions keep their bounds.reversed_reactionsandweightsare removed.fill_gaps_milpuses an integrality tolerance of 1e-9, as RAVEN does, and returnsexit_status="not_restored"when the selected reactions do not reach the floor in the filled model. An infeasible MILP on Gurobi is reported instead of raising.gap_reportraises aValueErrorwhen the model is infeasible under its own bounds or once its boundary is closed, naming the reactions with a forced flux and asking to set ATP maintenance to zero first.Curation and I/O
scale_biomassraises when the balancing component cannot absorb the change.Docs
reporter_metabolites,remove_no_flux_reactions, blocked reactions,check_tasksandfind_leak_metabolitedescribe the implemented behaviour; README and CONTRIBUTING links and claims corrected; raven-docs links point to CompMetEng.