Convert do.call calls to ParmOff by Copilot · Pull Request #16 · asgr/magicaxis

Copilot · 2026-05-14T12:42:28Z

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.call → ParmOff replacements (no behaviour change)

magaxis.R — axis (×6) and mtext (×2) calls
magplot.R — maghist, magmap, points, magbar calls
magbin.R — magmap (×3), magplot (×2), magbar calls
magimage.R — image call
magimageRGB.R — image 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.axis → magaxis, lty → magcon, pch → points).
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.

…gtri Agent-Logs-Url: https://github.com/asgr/magicaxis/sessions/b1365e3a-b831-4f58-913b-b006daa3c6c6 Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>

asgr

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

Agent-Logs-Url: https://github.com/asgr/magicaxis/sessions/e8cd5243-668b-401a-936b-63f792e6d411 Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>

Copilot · 2026-05-14T13:28:31Z

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.

… ways to envoke magaxis inside magplot since there are subtle issues with argument clashes.

…thub.com/asgr/magicaxis into copilot/convert-do-call-to-parmoff-again

Copilot

Pull request overview

This PR replaces internal do.call dispatch with ParmOff::ParmOff, adds package/test/vignette infrastructure for the new dependency, and expands magtri() argument routing so top-level ... can reach plotting subcalls.

Changes:

Adds ParmOff imports plus testthat/vignette dependencies.
Converts plotting helper dispatch in magaxis, magplot, magbin, magimage, magimageRGB, and magtri.
Adds ParmOff routing tests and a new long-form vignette.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`.gitignore`	Ignores `.positai`.
`DESCRIPTION`	Bumps package version and adds ParmOff/test/vignette dependencies.
`NAMESPACE`	Imports `ParmOff` and `points.default`.
`R/magaxis.R`	Replaces axis/mtext dispatch with ParmOff.
`R/magbin.R`	Replaces magmap/magplot/magbar dispatch with ParmOff.
`R/magcon.R`	Passes only x/y/z components into `magimage`.
`R/magimage.R`	Replaces image dispatch with ParmOff.
`R/magimageRGB.R`	Replaces image dispatch with ParmOff.
`R/magplot.R`	Replaces maghist/magmap/points/magbar dispatch with ParmOff.
`R/magtri.R`	Routes `...` through ParmOff to magaxis, magcon, and points calls.
`man/magaxis.Rd`	Removes an extra blank line.
`man/magmap.Rd`	Removes an extra blank line.
`tests/testthat.R`	Adds testthat entry point.
`tests/testthat/test-parmoff-routing.R`	Adds routing/rendering regression tests.
`vignettes/magicaxis.Rmd`	Adds a package vignette documenting plotting helpers and ParmOff routing.

Comments suppressed due to low confidence (1)

R/magplot.R:222

The standard scatter branch no longer passes ... into magaxis(), which regresses existing magplot(..., cex.axis=..., col.axis=..., cex.lab=...) usage for custom axes. Since the base plot() call has axes = FALSE, these axis styling arguments are not applied elsewhere.

              axis.col = axis.col

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    ParmOff(mtext, c(list(text=xlab, side=ifelse(side[1] %in% c(1,3), side[1], side[2]), line=mtline[1]), dots), .use_args = keepmtext)
  }
  if(is.null(ylab)==FALSE){
-    do.call("mtext", c(list(text=ylab, side=ifelse(side[2] %in% c(2,4), side[2], side[1]), line=mtline[2]), dotsmtext))
+    ParmOff(mtext, c(list(text=ylab, side=ifelse(side[2] %in% c(2,4), side[2], side[1]), line=mtline[2]), dots), .use_args = keepmtext)


                ticks.lwd = ticks.lwd,
-                axis.col = axis.col,
-                ...
+                axis.col = axis.col


+          dots_carry = ParmOff(magaxis, dots, side=1:2, grid=grid, grid.col='lightgrey', labels=FALSE, do.tick=do.tick, .pass_dots=FALSE, .return='func_args')$args_out$ignore_args
+          ParmOff(points.default, dots_carry, x=chains[usesamps,i], y=chains[usesamps,j], pch='.', col='darkgrey', .pass_dots=TRUE)
+          ParmOff(points.default, dots_carry, x=meanvec[i], y=meanvec[j], col='red', pch=4, cex=2, .pass_dots=TRUE)


+    expect_no_error(magaxis(side = 1:2, xlab = "X axis", ylab = "Y axis",
+                            cex.lab = 1.2, col.lab = "darkred"))


+test_that("magtri accepts pch forwarded to points without error", {
+  with_null_dev({
+    set.seed(10)
+    chains <- data.frame(x = rnorm(200), y = rnorm(200), z = rnorm(200))
+    expect_no_error(magtri(chains, samples = 100, pch = 16))
+  })


asgr

All good. I need to get ParmOff onto CRAN before magicaxis can be updated on CRAN.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

convert do.call to ParmOff; route dots to magaxis/magcon/points in ma…

289cb26

…gtri Agent-Logs-Url: https://github.com/asgr/magicaxis/sessions/b1365e3a-b831-4f58-913b-b006daa3c6c6 Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>

Copilot AI assigned Copilot and asgr May 14, 2026

Copilot created this pull request from a session on behalf of asgr May 14, 2026 12:43 View session

Copilot AI requested a review from asgr May 14, 2026 12:43

Copilot finished work on behalf of asgr May 14, 2026 12:43

asgr requested changes May 14, 2026

View reviewed changes

Copilot started work on behalf of asgr May 14, 2026 13:24 View session

add unit tests and vignette for ParmOff argument routing

523981a

Agent-Logs-Url: https://github.com/asgr/magicaxis/sessions/e8cd5243-668b-401a-936b-63f792e6d411 Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>

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 5 commits May 14, 2026 23:56

Some tidying up. Need to think about whether there are better (safer)…

021676d

… ways to envoke magaxis inside magplot since there are subtle issues with argument clashes.

ignore

ef5ec9f

Added majorn test

11da150

vignette edtis

fd124c4

Merge branch 'copilot/convert-do-call-to-parmoff-again' of https://gi…

fb00f29

…thub.com/asgr/magicaxis into copilot/convert-do-call-to-parmoff-again

asgr requested a review from Copilot May 16, 2026 05:16

Copilot started reviewing on behalf of asgr May 16, 2026 05:16 View session

Copilot AI reviewed May 16, 2026

View reviewed changes

asgr approved these changes May 16, 2026

View reviewed changes

asgr and others added 4 commits May 16, 2026 22:26

Potential fix for pull request finding

ef2340f

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Potential fix for pull request finding

9f778fc

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

edit

aa7f02c

Potential fix for pull request finding

fa05040

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

asgr marked this pull request as ready for review May 16, 2026 14:39

asgr merged commit f317723 into master May 16, 2026
3 checks passed

asgr deleted the copilot/convert-do-call-to-parmoff-again branch May 16, 2026 14:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert do.call calls to ParmOff#16

Convert do.call calls to ParmOff#16
asgr merged 11 commits into
masterfrom
copilot/convert-do-call-to-parmoff-again

Copilot AI commented May 14, 2026 •

edited

Loading

Uh oh!

asgr left a comment

Uh oh!

Copilot AI commented May 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asgr left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		expect_no_error(magaxis(side = 1:2, xlab = "X axis", ylab = "Y axis",
		cex.lab = 1.2, col.lab = "darkred"))

Conversation

Copilot AI commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

asgr left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented May 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asgr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 14, 2026 •

edited

Loading