Skip to content

Convert do.call calls to ParmOff#16

Draft
Copilot wants to merge 5 commits into
masterfrom
copilot/convert-do-call-to-parmoff-again
Draft

Convert do.call calls to ParmOff#16
Copilot wants to merge 5 commits into
masterfrom
copilot/convert-do-call-to-parmoff-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 14, 2026

Replaces all do.call calls across the package with ParmOff::ParmOff, and improves argument routing in magtri so that ... is forwarded to magaxis, magcon, and points instead of only magcon.

Changes

Package infrastructure

  • Added ParmOff to Imports in DESCRIPTION with a Remotes: asgr/ParmOff entry
  • Added importFrom("ParmOff", "ParmOff") to NAMESPACE
  • Added testthat (>= 3.0.0), knitr, and rmarkdown to Suggests
  • Added VignetteBuilder: knitr and Config/testthat/edition: 3

1-to-1 do.callParmOff replacements (no behaviour change)

  • magaxis.Raxis (×6) and mtext (×2) calls
  • magplot.Rmaghist, magmap, points, magbar calls
  • magbin.Rmagmap (×3), magplot (×2), magbar calls
  • magimage.Rimage call
  • magimageRGB.Rimage call

magtri.R — the main improvement

Previously, all ... passed to magtri() were forwarded exclusively to magcon. Now a dots = list(...) is collected once and routed via ParmOff to every sub-function call:

Call Before After
magaxis(…) (all panels) fixed args only fixed args + user ...
magcon(…) fixed args + all ... fixed args + user ... (ParmOff deduplicates)
points(…) fixed args only fixed args + user ...

This means callers can now do things like:

magtri(chains, cex.axis = 0.7, lty = c(1, 1, 1), pch = 16)
#  cex.axis goes to every magaxis call (axis tick labels)
#  lty      overrides the default contour line types in magcon
#  pch      overrides the marker style in points

ParmOff's argument merging ensures user-supplied values override the defaults while avoiding duplicate-argument errors that the old do.call(func, c(list(fixed), dots)) pattern would throw.

Tests and vignette

  • Added tests/testthat/test-parmoff-routing.R with 15 unit tests covering ParmOff argument routing through magaxis, magplot, magmap, magbin/plot.magbin, magimage, and magtri (including forwarding cex.axismagaxis, ltymagcon, pchpoints).
  • Added vignettes/magicaxis.Rmd, a long-form vignette with fig.width = 6 / fig.height = 6 throughout, covering magaxis, magplot, maghist, magcon, magbin, magimage, and magtri; includes a dedicated section demonstrating the new ParmOff-based argument routing in magtri.

Copy link
Copy Markdown
Owner

@asgr asgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add relevant unit tests and create a long form vignette magicaxis.Rmd that uses fig.width and fig.height of 6.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 14, 2026

Add relevant unit tests and create a long form vignette magicaxis.Rmd that uses fig.width and fig.height of 6.

Done in 523981a. Added:

  • tests/testthat/test-parmoff-routing.R — 15 unit tests covering ParmOff argument routing through magaxis (forwarding cex.axis/col.axis, filtering unknown args, xlab/ylab via mtext), magplot (basic, log, z-coloured, histogram dispatch), magmap, magbin/plot.magbin, magimage, and magtri (return structure, forwarding cex.axis to magaxis, lty to magcon, pch to points, refvals, custom lab).

  • vignettes/magicaxis.Rmd — long-form vignette with fig.width = 6 / fig.height = 6 throughout, covering magaxis, magplot, maghist, magcon, magbin, magimage, and magtri; includes a dedicated section on the new ParmOff-based argument routing in magtri.

Also updated DESCRIPTION with testthat, knitr, rmarkdown in Suggests, VignetteBuilder: knitr, and Config/testthat/edition: 3.

Copilot finished work on behalf of asgr May 14, 2026 13:29
Copilot AI requested a review from asgr May 14, 2026 13:29
asgr added 3 commits May 14, 2026 23:56
… ways to envoke magaxis inside magplot since there are subtle issues with argument clashes.
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.

2 participants