[Rd] all.equal() applied to function closures -- now committed

[Martin Maechler]

>>>>> Martin Maechler 
>>>>>     on Thu, 3 Dec 2020 18:29:58 +0100 writes:

>>>>> Martin Maechler 
>>>>>     on Tue, 1 Dec 2020 10:31:36 +0100 writes:

>>>>> Bill Dunlap 
>>>>>     on Mon, 30 Nov 2020 13:41:54 -0800 writes:

    >>> To make the comparison more complete,
    >>> all.equal.environment could compare the parents of the
    >>> target and current environments.  That would have to be
    >>> recursive but could stop at the first 'top level
    >>> environment' (the global, empty, or a package-related
    >>> environment generally) and use identical there.  E.g.,

    >>> > f1 <- function(x) (function(){ expx <- exp(x) ; function(y) y + expx})()
    >>> > all.equal(f1(2), f1(3))
    >>> [1] "Environments: Component “expx”: Mean relative difference: 1.718282"
    >>> 
    >>> [2] "Environments: <parent.env> Component “x”: Mean relative difference:
    >>> 0.5"

    >>> This is from the following, where I avoided putting the existing
    >>> non-recursive all.equal.environment into the body of this one.

    > [[.........]]

    >> Thank you, Duncan and Bill (and Kevin for bringing up the
    >> topic).

    >> I agree  all.equal() should work better with functions,

    >> and I think probably it would make sense to define  all.equal.function()
    >> rather than put this into all.equal.default()

    >> However, it's not quite clear if it is always desirable to check the
    >> environments as well notably as that *is* done recursively.

    > A small  "work in progress" update (because I've been advancing slowly only):

    > I'm currently testing 'make check-all' after

    > 1) adding all.equal.function() method,
    > 2) not changing  all.equal.environment() yet
    > 3) but adapting all.equal.default() to give a deprecating warning
    > when called with a function

    > all.equal.function <- function(target, current, check.environments = TRUE, ...)
    > {
    [........]
    > }

    > It's amazing that this breaks our own checks in several places,
    > which I'm chasing slowly (being busy with teaching related
    > duties, etc).
    > I did have the correct gut feeling to have 'check.environments'
    > being an optional argument, because indeed there are cases (in
    > our own regression tests) where we now needed to change the
    > checks to use   check.environments=FALSE .

    > My plan is to finish the   all.equal.function()  functionality /
    > modify "base R" coded (or, for now, rather the testing code)
    > where needed, and then *commit* that to R-devel.

I've done that now:
------------------------------------------------------------------------
r79555 | maechler | 2020-12-04 12:13:06 +0100 (Fri, 04 Dec 2020) | 1 line
Changed paths:
   M doc/NEWS.Rd
   M src/library/base/R/all.equal.R
   M src/library/base/R/zzz.R
   M src/library/base/man/all.equal.Rd
   M src/library/base/man/dput.Rd
   M src/library/stats/tests/nls.R
   M src/library/stats/tests/nls.Rout.save
   M tests/eval-etc-2.Rout.save
   M tests/eval-etc.Rout.save
   M tests/eval-fns.R
   M tests/reg-tests-1d.R

new all.equal.function() checks environment(.)
------------------------------------------------------------------------

Note the seven extra files needed to be changed

  src/library/base/man/dput.Rd
  src/library/stats/tests/nls.{R,Rout.save}
  tests/eval-{etc.Rout.save,fns.Rtests/eval-etc{,-2}.Rout.save}

where in all cases I needed to add an explicit
', check.environments = FALSE' to an existing  stopifnot(all.equal(..))
check.

Consequently, I expect some  "carnage" in checks of existing  CRAN / BioC 
(and github, etc but that does not count) packages.

The NEWS entry (in 'NEW FEATURES') mentions an important case :

    • all.equal(f, g) for functions now by default also compares their
      environment(.)s, notably via new all.equal method for class
      function.  Comparison of nls() fits, e.g., may now need
      all.equal(m1, m2, check.environments=FALSE).

So if you are a package maintainer observing that all.equal() --
or its corresponding 'testthat' obfuscation -- signals errors
"suddenly" (and only in R-devel), it's that you need to add
'check.environments=FALSE' there.

The good news is that this works also in existing (and previous)
versions of R :

  > all.equal(lm,lm, check.environments=FALSE)
  [1] TRUE
  > 

thanks to the "..."  _feature_ of typically just silently swallowing
everything that's not understood.  (yes: _feature_ is tongue in cheek).

Martin


    > After that we should come back to improve or even re-write
    > all.equal.environment()  when needed.