ocbe-uio / TruncExpFam

R package to generate data related to the Truncated Exponential Family
https://ocbe-uio.github.io/TruncExpFam/
GNU General Public License v3.0
0 stars 1 forks source link

Future-proof against breaking change on R-devel #103

Closed wleoncio closed 6 months ago

wleoncio commented 7 months ago

This was received on 2024-02-23 as a message forwarded by Luke Tierney from the R-devel mailing list:

This note was sent to R-devel last October. Your TruncExpFam nodbi is affected by this change and will start failing CRAN checks under R-devel shortly.

Best,

luke ---------- Forwarded message ---------- Date: Fri, 20 Oct 2023 19:56:20 +0000 From: luke-tierney@uiowa.edu To: R-devel@r-project.org Subject: UseMethod forwarding of local variables

UseMethod has since the beginning had the 'feature' that local variables in the generic are added to the environment in which the method body is evaluated. This is documented in ?UseMethod and R-lang.texi, but use of this 'feature' has been explicitly discouraged in R-lang.texi for many years.

This is an unfortunate design decision for a number of reasons (see below), so the plan is to remove this 'feature' in the next major release.

Fortunately only a small number of packages on CRAN (see below) seem to make use of this feature directly; a few more as reverse dependencies. The maintainers of the directly affected packages will be notified separately.

Current R-devel allows you to set the environment variable R_USEMETHOD_FORWARD_LOCALS=none to run R without this feature or R_USEMETHOD_FORWARD_LOCALS=error to signal an error when a forwarded variable's value is used.

Some more details:

An example:

 > foo <- function(x) { yyy <- 77; UseMethod("foo") }
 > foo.bar <- function(x) yyy
 > foo(structure(1, class = "bar"))
 [1] 77

Some reasons the design is a bad idea:

  • You can't determine what a method does without knowing what the generic it will be called from looks like.

  • Code analysis (codetools, the compiler) can't analyze method code reliably.

  • You can't debug a method on its own. For the foo() example,

    foo.bar(structure(1, class = "bar")) Error in foo.bar(structure(1, class = "bar")) : object 'yyy' not found

  • A method relying on these variables won't work when reached via NextMethod:

    foo.baz <- function(x) NextMethod("foo") foo(structure(2, class = c("baz", "bar"))) Error in foo.bar(structure(2, class = c("baz", "bar"))) : object 'yyy' not found

The directly affected CRAN packages I have identified are:

  • actuar
  • quanteda
  • optmatch
  • rlang
  • saeRobust
  • Sim.DiffProc
  • sugrrants
  • texmex

Some of these fail with the environment set to 'error' but not to 'none', so they are getting a value from somewhere else that may or may not be right.

Affected as revdeps of optmatch:

  • cobalt
  • htetree
  • jointVIP
  • MatchIt
  • PCAmatchR
  • rcbalance
  • rcbsubset
  • RItools
  • stratamatch

Affected as revdeps of texmex:

  • lax
  • mobirep

I'll study and try to reproduce the issue―so far our R-devel checks are fine, but as Luke said they are bound to fail soon―and release a fix ASAP.

wleoncio commented 7 months ago

Hard deadline on 2024-03-10, as the package is failing on CRAN:

Dear maintainer,

Please see the problems shown on https://cran.r-project.org/web/checks/check_results_TruncExpFam.html.

The ERRORs for r-devel are from

r85979 | luke | 2024-02-24 01:18:27 +0100 (Sat, 24 Feb 2024) | 4 lines Move towards UseMethod no longer forwarding local variables from the generic. For now they are forwarded with promises that signal an error i forced.

Please correct before 2024-03-10 to safely retain your package on CRAN.

Best, -k

wleoncio commented 7 months ago

Example of useful error message:

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-direct-sampling.R:7:3'): Original attributes are retrieved ─────
Error in `mget(ls())`: getting UseMethod variable 'parms' from generic 'rtrunc_direct'; this is no longer supported
Backtrace:
    ▆
 1. └─TruncExpFam::rtrunc(1e+06, mean = 1, sd = 2, faster = TRUE) at test-direct-sampling.R:7:3
 2.   ├─TruncExpFam::rtrunc_direct(n, family, ...)
 3.   └─TruncExpFam:::rtrunc_direct.normal(n, family, ...)
 4.     ├─TruncExpFam:::truncated_q(...)
 5.     │ └─base::paste0("trunc_", class(parms[["n"]]))
 6.     └─base::mget(ls())

See more here. Problem seems to be contained to gcc, as R-devel on clang doesn't fail. Regardless, must address within the fortnight.

wleoncio commented 7 months ago

To reproduce:

rhub::check_with_rdevel(env_vars = c("R_USEMETHOD_FORWARD_LOCALS" = "error"))
wleoncio commented 7 months ago

CI tests failing on develop but passing on release, so something probably went wrong in merging the hotfix branch.

wleoncio commented 7 months ago

Keep an eye on the CRAN check results for 1.1.1 before really closing this. Fix on develop is independent of all this.

wleoncio commented 6 months ago

All good, closing:

bilde