Repository navigation
Streamline unit tests (review draft for microbiome/mia#748) - #1
Draft
raivo-otus wants to merge 31 commits into
Draft
raivo-otus wants to merge 31 commits into
raivo-otus wants to merge 31 commits into
Conversation
Defines gp_small in a testthat helper so heavy tests can run on a small, seeded subset of GlobalPatterns with a pruned rowTree instead of the full 19216-feature object. Sourced before every test file by test_check() and devtools::test(). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… rounds test-addAlpha.R ran ~100 rarefaction passes over the full 19216-feature GlobalPatterns object (245 s locally, the single largest test cost). The block now uses agglomerateByRank(GlobalPatterns, "Family") (341 features at full sequencing depth) and niter = 3 / 5 instead of 10 / 20, taking ~11 s. All expectations are kept. A random feature subset was tried first but breaks the richness-estimator sanity checks, so a taxonomic aggregation is used instead. Two correlation thresholds that are sanity checks on the data rather than on the code were lowered to what family-level data supports (camargo 0.7 -> 0.5, absolute 0.9 -> 0.8); all other tolerances are unchanged. Also fixes a copy-paste slip that checked for pielou_10 in the chao1 block, and hoists the rarefaction depth into one variable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The block fetched microbiomeDataSets::peerj32() from ExperimentHub with a 5x retry and silently skipped all of its assertions when the download failed. It now uses the bundled HintikkaXOData (top 50 genera x 38 metabolites), so the test needs no network and always runs. Coverage is unchanged and the numeric check is stronger: instead of nine hard-coded numbers from microbiome::associate, every correlation, p-value and adjusted p-value is compared with stats::cor.test / p.adjust computed on the same pairs. Also: - the 26 argument-validation calls share one base argument list; this fixes a call that referred to an undefined object (mae1) and therefore passed for the wrong reason - the wall-clock assertion (symmetric run faster than the full run) is dropped; the equality of the two results is kept - the duplicate-name check reuses the table computed for the symmetric check instead of recomputing it - splitByRanks is restricted to the two ranks the assertions use - random draws are seeded Local run time 59 s -> 16 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
- the getRare rank loop agglomerated the full object three times for each of the seven ranks; the prevalent/rare partition does not depend on the rank, so it now runs for Phylum and Genus on the gp_small fixture - subsetByPrevalent and subsetByRare had structurally identical blocks; they are now one parameterised block run for both functions, on gp_small - the random detection/prevalence union check is seeded and also asserts that the union equals all taxa (it only checked a subset relation) - byte-identical duplicate calls and alias self-comparisons (getTop vs getTop, agglomerateByPrevalence vs the same call) are removed - the na.rm check compared a vector with itself; it now compares against the reference, and removes the NA features by name instead of by an index vector that never matched - the miaTime block is guarded with skip_if_not_installed, uses a fixed threshold (was runif) and a seeded sample, and computes each agglomeration once (four calls -> two) Assertions on dims and specific features stay on the full GlobalPatterns. Local run time 55 s -> 29 s (the remaining cost is the fixed per-call cost of agglomerateByRank). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The block split and unsplit the full 19216-feature object several times, including a split by Phylum into ~67 groups. It now uses gp_small; the Phylum round trip uses three phyla since the assertion (assay equality after unsplit) does not depend on the number of groups. The loop that checks the tip count of every updated tree iterated over 'length(list)' and therefore only checked the last element; it now iterates over seq_along(list). Local run time 56 s -> 12 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The block fitted the same k = 2 model three times (addNMF, NMF::nmf and getNMF) to compare loadings; the getNMF comparison now reads the loadings stored by addNMF, which calls getNMF. The nrun = 2 calls on both the mia and the NMF side pass .options = "-p" so that NMF does not spin up its parallel backend; this removes the dependence on the core count of the build machine and is the likely reason the test was much slower inside the full suite than alone. Local run time 50 s -> 25 s (NMF itself is the remaining cost). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…SEs tests Six merges involved the full 19216-feature GlobalPatterns with tree reconciliation; the assertions (tree identity, node labels, rowData completeness, colData column count, assay names) hold for any subset, so they now use gp_small. Two expect_error calls referred to an undefined object and passed for the wrong reason; they now pass a three-element list, which is what the left/right join check is about. The data() reloads that undid earlier in-place edits are replaced by local copies, the random colData groups are seeded, and the miaTime block is guarded with skip_if_not_installed. Local run time 32 s -> 15 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The transformation test defined a helper that was called twice (matrix and DelayedArray assay) on the full GlobalPatterns object. The helper reloaded GlobalPatterns part-way through, so the philr, cutoff, binning, difference, division and invnorm checks ran twice on identical input. Those checks now live in their own blocks and run once; philr has its own skip-guarded block. The helper runs on gp_small for both storage types. The CSS comparison with metagenomeSeq reference values depends on the whole matrix, so it stays on the full object (once, by feature and sample name instead of by position). Hand-written relabundance, hellinger and clr recomputations are replaced by the vegan::decostand comparisons that already existed further down (the direction of PR microbiome#749); altExps are built for the two ranks the assertions use instead of all seven; the random rank pseudocount is seeded. Local run time 35 s -> 9 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Drops a byte-identical duplicate Family agglomeration (its extra assertion is kept on the previous result), runs the empty.rm, warning, empty.ranks.rm and agglomerateByModule checks on gp_small (with 5 seeded random modules instead of 30 over 19216 features), and keeps the Species-level counts and make.unique checks on the full object because their expected values are specific to it. The miaTime block is guarded with skip_if_not_installed, computes the update.refseq = FALSE agglomeration once (was twice) and uses a fixed threshold and a seeded feature instead of runif/sample. Local run time 28 s -> 15 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The hand-computed references do not depend on the data size, so the mockSCE object is 300 x 40 instead of 2000 x 200, the na.rm checks use gp_small (seeded random indices), and the two-tree merge uses gp_small instead of the full GlobalPatterns. The check that compared agglomerateByVariable with itself is replaced by comparing it with the .merge_features result computed for the same arguments, and the duplicate computation of that result is removed. Local run time 15 s -> 5 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…test IdTaxaToDataFrame only converts the classification result, so 20 of the DECIPHER example sequences are enough. The lowest.rank label checks run on gp_small; addHierarchyTree and mapTaxonomy keep the full object because their expected values (node counts, genera) are specific to it. Local run time 18 s -> 10 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
agglomerateByRanks on the full object is needed once for the expected dimensions; the comparison with splitByRanks is now restricted to the two ranks that were compared anyway instead of recomputing all seven. Local run time 16 s -> 7 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The block fitted four LDA models on the full 19216-term matrix, one of them with a random number of topics. It now agglomerates to genus level first (the comparison with topicmodels::LDA is unchanged), reads the loadings stored by addLDA instead of refitting via getLDA, and checks perplexity and coherence on the model already fitted. Local run time 14 s -> 2 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The block converted the full GlobalPatterns object to phyloseq several times, agglomerated it at all seven ranks, and merged it with esophagus. The assertions compare slots of the phyloseq object with the input and hold for any subset, so the block now uses gp_small; the rank loop keeps one rank with few and one with many missing values; the duplicate conversion used for the factor-rowData comparison reuses the first. Local run time 25 s -> 9 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three expect_equal calls had the closing parenthesis in the wrong place, so the reference vector was rounded to integers and the digit count became the tolerance; every comparison passed regardless of the value. The comparisons are now made at the precision of the reference values. The random reference samples are seeded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The tree.name validation checks only need an object with two rowTrees, so mergeSEs(gp_small, esophagus) replaces the full-object merge. The Faith comparison with picante keeps the full GlobalPatterns. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
meltSE on the 19216 x 26 object was called seven times to check column names and warnings; gp_small gives the same columns. The getTop block keeps the full object because its expected taxa are specific to it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dive The skip_if_not for ecodive came after the first ecodive call, so a machine without ecodive errored instead of skipping. The comparisons between mia and ecodive hold for any tree, so they run on gp_small. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The closing parenthesis put the digit count into tolerance and rounded the reference to integers; the comparison now rounds both sides. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Three expect_equal calls rounded the reference to integers and used the digit count as tolerance; they now round both sides to seven digits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The reference was rounded to integers with a tolerance of 7; both sides are now rounded to seven digits. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The merge of esophagus with the full GlobalPatterns fed two assertions that have been commented out; nothing used its result. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The comparison of the variables and formula interfaces does not depend on the data, so it runs on gp_small instead of the full object. Permutation counts are unchanged because addCCA forwards 'permutations' only to some of its internal vegan calls. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…pping The block compared two byte-identical getMDS calls. It now checks that a named node.label vector gives the same ordination as the default mapping through rowLinks. (A plain positional vector gives a different ordination through getMDS although getDissimilarity agrees for both; that is left as is and noted for the maintainers.) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The direct comparison read a non-existent aov.tab element from both results, so it compared NULL with NULL. With the same seed the two permutation tests are identical, so the whole anova table is compared. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…andom index choice Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The helper was disabled in 2024 because of an as.matrix() method mismatch; it passes now for both matrix and DelayedArray assays, adding 22 expectations. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Both were compared with themselves. getTop is now compared with the top-5 row means computed directly, and the dominance summary with the per-sample dominant taxa from addDominant. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
inst/extdata/taxpasta_no_type.biom was a 619 KB HDF5 BIOM with 15352 observations; the only test that uses it asserts the class of the imported object, yet spent 16 s parsing row-wise observation metadata and agglomerating eight ranks. The file is now a 36 KB subset of the same taxpasta output with 40 observations chosen so that every one of the 31 taxonomy columns is populated, written with rhdf5 to the same HDF5 layout (empty 'type' attribute included). The derivation script is attached to the pull request. Local run time of test-importTaxpasta.R: 16 s -> 3 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ecks - test-5addLDA.R: topicmodels::LDA seeds from the clock by default, and on genus-level data the fit depends on the seed, so the comparison with addLDA passed only when both fits started within the same second (3 of 30 local runs failed). Both fits now use control = list(seed = 123); 40 of 40 repeated runs pass. - test-5addLDA.R, test-5addNMF.R: the getLDA/getNMF checks read the loadings with getReducedDimAttribute, which is the attribute already compared on the previous line. They are removed; getLDA and getNMF are still run through addLDA and addNMF. The remaining NMF comparison keeps the stricter tolerance (1e-4) of the removed one. - test-5getCrossAssociation.R: the col.var1/col.var2 overrides failed on the "only one of assay.type, col.var and dimred" check because the base arguments set assay.type1 and assay.type2. They now clear the matching assay.type, so the error comes from the col.var validation itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Why
Bioconductor devel (3.24) check of mia 1.21.7, 2026-09-15:
testthat.R1404 stestthat.Rtestthat.RThe unit tests are ~70 % of the check time. Release 3.23 (1.20.0) already times out on the aarch64 node.
What changed
Only
tests/testthat/and one test fixture (inst/extdata/taxpasta_no_type.biom, used by a single test). No change underR/.tests/testthat/helper-data.Rdefinesgp_small, a seeded 300-feature subset ofGlobalPatterns(rows kept in tree order, rowTree pruned to 300 tips). Tests whose assertions are structural (dims, names, classes, warnings) or compare mia with an upstream package (vegan, ecodive, phyloseq, NMF, topicmodels, base R) run on it; tests with hard-coded expected values derived from the full object keep the full object.niter3/5 instead of 10/20 over 19216 features; LDA on genus-level data;getCrossAssociationon the bundledHintikkaXODatainstead of a network fetch ofmicrobiomeDataSets::peerj32();mockSCE(40, 300); IdTaxa on 20 sequences; a 40-observation taxpasta BIOM fixture..options = "-p").expect_equal(round(x, 7), round(ref), 7)with the closing parenthesis in the wrong place, so the reference was rounded to integers and the digit count became the tolerance; twoexpect_errorcalls passed because they referred to an undefined object; a loop overlength(list)checked only the last element;getPERMANOVAwas compared withadonis2through a non-existentaov.tabelement (NULL vs NULL);getTopandsummarizeDominancewere compared with themselves; random draws are now seeded; the internal richness helper disabled in 2024 passes again and is re-enabled (+22 expectations); optional packages (miaTime,ecodive) are guarded withskip_if_not_installed()before first use.One commit per test file; each commit message states what was removed and what still covers the same behaviour.
Timing (local, 16-core Linux; the Bioc Linux node is roughly 2x slower)
Before: 708 s (all Suggests installed). After: **190 s (3.7x faster; 118 blocks, 0 failures, 1 skip for dada2). The tests part of a local
R CMD check([ FAIL 0 | WARN 5 | SKIP 1 | PASS 1376 ], same five pre-existing vegan warnings as before) took 202 s on a loaded machine; the Bioc Linux node needed 1404 s before. With the optional packages (NMF, topicmodels, mediation, reldist, topicdoc, philr, miaTime, dada2) absent, all their blocks skip and nothing errors (9 skips, 0 failures)**.Coverage
covr::package_coverage()before/after on the same machine: line coverage 74.85 % -> 74.97 % (545 functions). Per function: no function lost a covered line, six gained (the re-enabled richness helper covers.calc_ace,.calc_chao1,.calc_hill; plus lines inmergeSEs,getCrossAssociationand.check_altExp_present).Notes for reviewers
test-addAlpha.R: two correlation sanity thresholds were lowered (camargo 0.7 -> 0.5, absolute 0.9 -> 0.8) because they are properties of the full data, not of the code; all other tolerances are unchanged. A random feature subset breaks the richness-estimator checks entirely, which is why family-level aggregation is used there.test-addMDS.R: while fixing a self-comparison it turned out thatgetMDS(..., node.label = <unnamed vector>)gives a different ordination than the default mapping, althoughgetDissimilarity()agrees for both. The test now checks the named-vector case (which matches the default). The positional case may deserve a look inR/.agglomerateByRank(),splitOn()/unsplitOn()andagglomerateByRanks()regardless of data size; that is package code and out of scope here.nruntreatment would apply to them in a follow-up.Supersedes the draft microbiome#749.
Profiling script (run against an installed build)
Fixture derivation (
inst/extdata/taxpasta_no_type.biom)🤖 Generated with Claude Code