Remove importExcelModel and convert tutorial models to YAML#631
Merged
Conversation
- Remove io/importExcelModel.m and its generated documentation - Convert tutorial models small, smallYeast and smallYeastBad from Excel to YAML; rewrite tutorials 1-4 and their solutions to load models via importModel/readYAMLmodel instead of importExcelModel - Drop the smallYeastBad2 section of tutorial 4 (duplicate metabolite names cannot be represented in SBML/YAML) - readYAMLmodel: keep rxnGeneMat dimensionally consistent for gene-less models, which were left with an empty matrix that broke removeReactions - followChanged: handle the case where no reactions differ, where the empty reaction list was expanded to all reactions by constructEquations - fillGaps: guard against an empty toCheck list, which haveFlux otherwise interprets as all reactions - tutorial3_solutions: set upper bounds before lower bounds so a lower bound never transiently exceeds the previous upper bound - tutorial4: define the reference model before it is used - Remove Excel import coverage from tIO and importExportTests
Function test results0 tests 0 ✅ 0s ⏱️ Results for commit 8dc575a. |
With importExcelModel gone, nothing can read the Excel source models, so
the now-unreferenced tutorial/{small,smallYeast,smallYeastBad,
smallYeastBad2}.xlsx and "iAL1006 v1.00.xlsx" are removed. The small,
smallYeast and smallYeastBad models remain as YAML; iAL1006 remains as
SBML. The empty.{xml,xlsx,yml,mat} set is kept on purpose, as each format
is used to test a different importer (importModel, loadWorkbook/loadSheet,
readYAMLmodel/parseYAML and the MAT-based importExportTests). Update the
tutorial1 header that referred to the removed Excel file.
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.
Summary
Removes
importExcelModeland all references to it. Tutorial models that were only available in Excel are converted to YAML, and the tutorials are rewritten to load models viaimportModel/readYAMLmodel.Changes
io/importExcelModel.mand its generated documentation. No.mfiles reference it anymore.small,smallYeastandsmallYeastBadfrom Excel to YAML; rewrote tutorials 1–4 and their solutions to load viaimportModel/readYAMLmodel.smallYeastBad2section of tutorial 4 — its duplicate metabolite names cannot be represented in SBML/YAML (onlyimportExcelModelwithignoreErrorscould load it).tIOandimportExportTests.Pre-existing bugs fixed along the way
These were surfaced by running the tutorials end-to-end; each was verified to fail identically with the original Excel-loaded model (i.e. not caused by the conversion):
readYAMLmodel: gene-less models received a[0×0]rxnGeneMat, inconsistent with the reaction count, which brokeremoveReactions(and henceprintFluxes(...,true)). Now sized[nRxns×nGenes].followChanged: when no reactions differ, the empty reaction list was expanded to all reactions byconstructEquations, causing an out-of-bounds index.fillGaps: an emptytoChecklist was passed tohaveFlux, which interprets an empty reaction list as all reactions, causing an out-of-bounds index.tutorial3_solutions: lower bounds were set before upper bounds, so a lower bound transiently exceeded the previous upper bound.tutorial4: the reference model was used before being defined.Verification
All tutorials run end-to-end (MATLAB R2024b). Remaining non-passes are environment-only:
qMOMAin tutorial 3 needs the Optimization Toolbox (quadprog), and tutorial 4 needs a MILP-capable solver.Function tests green:
tIO17/0,tAnalysis18/0,tGapfilling9/0,importExportTests6/0.Note
The orphaned tutorial
.xlsxmodel files (small.xlsx,smallYeast.xlsx,smallYeastBad.xlsx,smallYeastBad2.xlsx,empty.xlsx,iAL1006 v1.00.xlsx) are left in place; withimportExcelModelgone, no RAVEN function reads them. They can be removed separately if desired.