[R] environments: functions within functions

[Ivan Krylov]

В Thu, 25 May 2023 10:18:13 -0400
Sarah Goslee <sarah.goslee using gmail.com> пишет:

> print called on this object gets passed to print.mixfitEM(), which is:
> 
> 
> function (x, digits = getOption("digits"), ...)
> {
>     family <- x$family
>     mc <- match.call()
>     mc$digits <- digits
>     fun.name <- paste0("print", family)
>     mc[[1]] <- as.name(fun.name)
>     eval(mc, environment())
> }
> 
> 
> Working through the calls, when eval() is called from within
> funfails(), mc is printnormal(x = thisx, digits = 7)
> and the calling environment does not contain thisx.

Your functions, both funworks and funfails, did nothing wrong. They are
using R as intended, so there shouldn't be anything to fix. I think that
mixR::mixfitEM is making a mistake in its use of non-standard
evaluation.

When working with match.call(), the typical pattern is to eval() the
modified call in the parent.frame() (where the call had originated and
where, presumably, all its referenced variables still live). mixR
cannot use this pattern unaltered because they want to call an
unexported function, e.g., mixR:::printnormal. The authors could
construct a call to ::: and insert that into mc[[1]] instead of
as.name(fun.name), so that the resulting call would be to
mixR:::printnormal(remaining arguments) and would thus cleanly evaluate
in the calling environment, but R CMD check could give them a NOTE for
using ::: (even with their own package).

One way to get around this would be to put the function itself in the
first element of the call (i.e. mc[[1]] <- get(fun.name) instead of
as.name(fun.name)), thus also making it possible to perform the call
despite as.name(fun.name) cannot be resolved from parent.frame(). This
could lead to scary-looking tracebacks.

Another way would be to keep the environment of the call as it is, but
evaluate the arguments from the matched call, something like:

for (i in seq_along(mc)[-1]) mc[[i]] <- eval(mc[[i]], parent.frame())

This code is untested, but the idea is to remove any dependency of the
matched call on the calling frame, thus making it possible to evaluate
it without problems in the package environment. This will again lead to
scary-looking tracebacks, potentially worse than the previous option
(depending on whether deparse() is longer for mixR:::printnormal or its
typical arguments), and will also wreak havoc on any additional use of
non-standard evaluation by printnormal (hopefully there isn't any, but
any arguments that should have stayed quoted will be evaluated instead).
They already do something like this with the digits=... argument.

I could also suggest a redesign of the package to make fuller use of
the S3 dispatch system (i.e. prepend x$family to class(x), possibly
with a package-specific prefix and make printnormal() and friends into
S3 methods for print()) instead of trying to implement it oneself, but
I understand that it may be not an available option.

I was going to suggest patching around the bug using trace(), but it
doesn't seem to work well with S3 method dispatch. No matter what I do,
UseMethod() seems to pick up the original definition of
mixR:::print.mixfitEM instead of the trace()-altered version:

trace(mixR:::print.mixfitEM, quote({ mc$x <- x }), at = 7)
mixR:::print.mixfitEM(fit1) # works as expected, but that doesn't matter
print(fit1) # tracer not called

-- 
Best regards,
Ivan