Skip to content

Testing suite overhaul#98

Open
grantdadams wants to merge 20 commits into
devfrom
testing-suite-overhaul
Open

Testing suite overhaul#98
grantdadams wants to merge 20 commits into
devfrom
testing-suite-overhaul

Conversation

@grantdadams

Copy link
Copy Markdown
Owner

Moved all plotting functions to standardized ggplot2 that plots, optionally saves, and returns ggplot2 object
Standardized testing following @Bai-Li-NOAA recommendations
This was a mix of Claude and my edits

Addresses the testing-suite recommendations in discussion #75.

Structure & robustness:
- Flatten the themed tests-*/ subfolders into a flat tests/testthat/ with
  test-<theme>-* names (testthat only auto-discovers the top level). Delete
  the fragile source()-based test-subdir-folders.R runner.
- Fix the skip policy: pure-R unit tests run everywhere; the fit_mod()
  integration tests carry a file-top skip_on_cran() so R CMD check stays
  CRAN-fast while the coverage job (NOT_CRAN=true) runs the full suite. This
  stops ~53 files being silently skipped in CI.
- Fix the latent expect_all_true(): it was called 52x but never defined.
  Add it as a custom expectation in helpers.R and de-qualify the calls.

Docs:
- Add tests/testthat/README.md: theme -> source map, run/skip/snapshot/
  coverage instructions, and the FIMS-style test-file template.
- Document the multi-species operating-model simulator and the calc_*_nll
  reference helpers.

Coverage & CI:
- Add covr to Suggests; add a TMB-patched test-coverage workflow (system
  deps + NOT_CRAN=true so the full suite counts) uploading to Codecov.
- Add the dev branch to the R-CMD-check triggers; add coverage/check badges.

Verified: full suite 3744 pass / 0 fail (NOT_CRAN=true); R CMD check tests OK
with 0 errors. Coverage baseline ~52%.
- test-plot-smoke.R: fit a small single- and multi-species model once and
  assert every exported plotter (plot_biomass/ssb/recruitment/depletion/
  index/catch/F/selectivity/mortality/data/comp, the plot() S3 method, and
  the predation plotters) runs without error, drawing to a throwaway device.
  This is the regression net for the upcoming base->ggplot rewrite.
- plot_mortality: replace deprecated ggplot2 size= with linewidth= in
  element_rect()/geom_contour()/geom_line() (ggplot2 >= 3.4.0).

Verified: 21 pass / 0 fail / 0 warn (NOT_CRAN=true).
First batch of the plotting overhaul (base graphics -> ggplot2,
standardized + colourblind-safe).

- R/7-plot_helpers.R: shared infrastructure for all plotters -- standard
  theme_classic-based theme, viridis colour/fill scales, model-list
  coercion, ggsave helper, and the unified "return the ggplot object"
  contract (so plots are composable and their data is testable).
- plot_timeseries (engine for plot_biomass/ssb/recruitment/depletion/
  ssb_depletion/exploitable_biomass): replace the base-graphics multi-panel
  renderer with a faceted ggplot built from a tidy data frame. The exact
  quantity/CI extraction is unchanged, so plotted values are identical; the
  function now returns the ggplot. Drop the vestigial .save_par() call.
- tests/testthat/test-plot-accuracy.R: assert each plotter's $data equals
  the same quantity from as.data.frame.Rceattle() (the canonical tidy
  output), aligned on the plotted years.

Verified: plot accuracy + smoke tests pass (NOT_CRAN=true); plotted biomass
matches source/1e6 exactly.
Batch 2 of the plotting overhaul.

- plot_index / plot_catch: rewrite the base-graphics survey-index and
  fishery-catch fit plots as faceted ggplots (observed points + lognormal
  95% bars, predicted line per model). Shared assembly (.fleet_fit_df) and
  renderer (.render_fleet_fit) remove the duplicated extraction/plot loops.
  plot_catch keeps MSE-projection ribbons. Both return the ggplot.
- plot_index gains a `log` argument; plot_logindex is now a deprecated
  thin forwarder to plot_index(log = TRUE) (drops ~220 duplicated lines).
- plot_indexresidual: rewrite the "pearson" branch as a ggplot of
  log(predicted/observed) by year (faceted by fleet); the "osa" branch
  still routes to osa_residuals()/plot.rceattle_osa().
- Old base-graphics-only arguments (line_col/right_adj/top_adj/single.plots/
  alpha/lwd/ymax) are retained as ignored args for back-compatibility.
- Tests: accuracy (plotted Predicted == quantities$index_hat / catch_hat)
  and smoke (incl. plot_index(log) and the plot_logindex deprecation).

Verified: plot accuracy + smoke tests pass (NOT_CRAN=true); index/catch
predictions match source exactly.
- plot_maturity: faceted ggplot of maturity-at-age (line per model), theme
  + viridis, returns the ggplot.
- Bug fix: the function read data_list$pmature, which does not exist (the
  field is data_list$maturity), so it errored on real fits. Now reads the
  correct field. It had no test coverage, which is why this went unnoticed.
- Tests: smoke + accuracy (plotted Maturity == data_list$maturity).

Part of batch 3 of the plotting overhaul; the persp-based selectivity
surfaces follow.
Batch 3 of the plotting overhaul. Selectivity/M age x year surfaces are now
drawn as year-coloured at-age curves (faceted by fleet) instead of 3D persp
-- the "multi-line by year" convention from r4ss's SSplotSelex, and far more
legible/accessible.

- plot_selectivity: year-coloured selectivity-at-age curves, faceted by
  fleet, sex as linetype. Returns the ggplot.
- plot_selectivity_vs_maturity: terminal-year fishery selectivity vs input
  maturity-at-age, faceted by species (SPR-debugging view).
- Tests: smoke + accuracy (terminal selectivity == sel_at_age).

Conventions cross-checked against r4ss / WHAM / SAM.
Batch 4 (part 1) of the plotting overhaul.

- plot_mortality: default to year-coloured M-at-age curves faceted by
  species (type = "lines"), with type = "heatmap" for the age x year tile.
  Drops the base-graphics persp/contour branches; returns the ggplot.
  M2 = TRUE plots M1 + M2, FALSE plots M1; `log` and `maxage` retained.
- Tests: accuracy (plotted M == quantities$M2_at_age).
plot_f was a near-duplicate of plot_timeseries for F_spp. It is now a thin
wrapper: plot_timeseries(output = "F_spp") for the line + CI machinery, plus
Ftarget/Flimit reference lines per species. Removes ~225 duplicated lines and
gives F the same consistent 95% CI band convention. Returns the ggplot.

Tests: F accuracy cross-checked against as.data.frame()$F_spp.
Completes batch 4 of the plotting overhaul.

- plot_m_at_age: total M (M1 + M2) at a fixed age as a time series, faceted
  by species (line per model, sex as linetype). Returns the ggplot.
- plot_m2_at_age_prop: proportion of predation mortality (M2) on each prey
  age attributable to each predator, coloured by predator and faceted by
  prey species/sex. The M2_prop extraction is unchanged. Returns the ggplot.
- Tests: smoke (multispecies) + accuracy (plot_m_at_age == M_at_age).
- plot_b_eaten: total biomass eaten as prey per species as a faceted
  time series (MSE ribbons when applicable). Returns the ggplot.
- plot_b_eaten_prop: biomass eaten of each prey by predator, coloured by
  predator and faceted by prey species. Returns the ggplot.
- plot_ration: population consumption (ration x biomass-at-age, summed over
  age) as a faceted time series. Explicit sex indexing fixes the original
  base-graphics drop bug (rowSums on a 1-sex slice). Returns the ggplot.
- plot_diet_comp (already ggplot): now returns the per predator/prey/year
  cowplot grids invisibly. Verified it plots observed
  (Stomach_proportion_by_weight) vs fitted (diet_hat[,2]) diet proportions,
  paired row-for-row via residuals(source = "diet").
- Tests: smoke (multispecies) + accuracy (plot_b_eaten == sum B_eaten_as_prey).
…gplot

Final batch of the plotting overhaul.

- plot_stock_recruit: SSB vs recruitment points + fitted SRR curve (mean /
  Beverton-Holt / Ricker), faceted by species, with a 95% normal data
  ellipse (add_ci). Returns the ggplot.
- plot_data: r4ss-style data-availability timeline rebuilt as a faceted
  ggplot (year x fleet, faceted by data type, point area = relative
  quantity). Accepts a fit or a raw data_list. Returns the ggplot.
- plot_form: functional-response forms drawn with geom_line (1-D forms) or
  geom_raster (2-D predator x prey), returning the ggplot. (Needs a
  multispecies params object, as before.)
- Tests: smoke (plot_data, plot_stock_recruit) + accuracy
  (plot_stock_recruit SSB/R == quantities$ssb/R).
devtools::document(): update man pages whose roxygen changed (plot_catch,
plot_index, plot_indexresidual, plot_logindex, plot_mortality,
plot_selectivity_vs_maturity, plot_stock_recruit) and add man pages for the
new internal plotting helpers (.as_model_list, .model_labels, .rceattle_theme,
.rceattle_scale, .save_ggplot). NAMESPACE unchanged.
plot_logindex was a thin scale variant of plot_index. Now that plot_index
takes log = TRUE, remove plot_logindex entirely rather than keeping a
deprecated shim: drop the function + its export/man page, update the
diagnostics and model-options vignettes, and drop the smoke-test case.

Note: this removes an exported function from a released package; callers
should switch to plot_index(..., log = TRUE).
The base-graphics index/catch fits used gplots::plotCI for error bars; the
ggplot rewrite uses geom_errorbar, so gplots is no longer used. Removing it
from Imports clears the "declared Imports should be used" R CMD check NOTE.
- Version 4.6.0 -> 4.7.0.
- NEWS.md: new 4.7.0 section covering the ggplot plotting overhaul (return
  ggplot objects, viridis/theme_classic, plot_index(log=), SR ellipses, the
  test-suite/coverage work), the breaking changes (plot_logindex removed,
  plotters return ggplot, gplots dropped), and the bug fixes.
- model-options vignette: note plotters are ggplot2-based and return the
  ggplot object (composable, optionally saved via file=).
Add *.gcda / *.gcno (generated by covr::package_coverage()) to .gitignore so
the coverage workflow's instrumentation output is never accidentally committed.
(Also picks up the .mcp.json ignore already pending in the working tree.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant