R devel - Jun 2019 - methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

>>>>> Martin Maechler 
>>>>>     on Sat, 29 Jun 2019 10:33:10 +0200 writes:
>>>>> peter dalgaard 
>>>>>     on Fri, 28 Jun 2019 16:20:03 +0200 writes:
    >> > On 28 Jun 2019, at 16:03 , Martin Maechler <maechler at
stat.math.ethz.ch> wrote:
    >> > 
    >> >>>>>> Henrik Bengtsson 
    >> >>>>>>    on Thu, 27 Jun 2019 16:00:39 -0700
writes:
    >> > 
    >> >> Using:
    >> >> 
    >> >> untrace(methods::conformMethod)
    >> >> at <- c(12,4,3,2)
    >> >> str(body(methods::conformMethod)[[at]])
    >> >> ## language omittedSig <- omittedSig &&
(signature[omittedSig] != "missing")
    >> >> cc <- 0L
    >> >> trace(methods::conformMethod, tracer = quote({
    >> >>  cc <<- cc + 1L
    >> >>  print(cc)
    >> >>  if (cc == 31) {  ## manually identified
    >> >>    untrace(methods::conformMethod)
    >> >>    trace(methods::conformMethod, at = list(at), tracer =
quote({
    >> >>      str(list(signature = signature, mnames = mnames,
fnames = fnames))
    >> >>      print(ls())
    >> >>      try(str(list(omittedSig = omittedSig, signature =
signature)))
    >> >>    }))
    >> >>  }
    >> >> }))
    >> >> loadNamespace("oligo")
    >> >> 
    >> >> gives:
    >> >> 
    >> >> Untracing function "conformMethod" in package
"methods"
    >> >> Tracing function "conformMethod" in package
"methods"
    >> >> Tracing conformMethod(signature, mnames, fnames, f, fdef,
definition)
    >> >> step 12,4,3,2
    >> >> List of 3
    >> >> $ signature: Named chr [1:4] "TilingFeatureSet"
"ANY" "ANY" "array"
    >> >>  ..- attr(*, "names")= chr [1:4]
"object" "subset" "target" "value"
    >> >>  ..- attr(*, "package")= chr [1:4]
"oligoClasses" "methods" "methods"
"methods"
    >> >> $ mnames   : chr [1:2] "object"
"value"
    >> >> $ fnames   : chr [1:4] "object"
"subset" "target" "value"
    >> >> [1] "f"          "fdef"      
"fnames"     "fsig"       "imf"
    >> >> [6] "method"     "mnames"    
"omitted"    "omittedSig" "sig0"
    >> >> [11] "sigNames"   "signature"
    >> >> List of 2
    >> >> $ omittedSig: logi [1:4] FALSE TRUE TRUE FALSE
    >> >> $ signature : Named chr [1:4] "TilingFeatureSet"
"ANY" "ANY" "array"
    >> >>  ..- attr(*, "names")= chr [1:4]
"object" "subset" "target" "value"
    >> >>  ..- attr(*, "package")= chr [1:4]
"oligoClasses" "methods" "methods"
"methods"
    >> >> Error in omittedSig && (signature[omittedSig] !=
"missing") :
    >> >>  'length(x) = 4 > 1' in coercion to
'logical(1)'
    >> >> Error: unable to load R code in package 'oligo'
    >> >> 
    >> > 
    >> > Thank you, Henrik, nice piece of using trace() .. and the
above
    >> > is useful for solving the issue --  I can work with that.
    >> > 
    >> > I'm  already pretty sure the wrong code starts with
    >> > 
    >> >    omittedSig <- sigNames %in% fnames[omitted] # ....

    > my  "pretty sure"  statement above has proven to be wrong ..

    >> > -------------
    >> > 
    >> 
    >> I think the intention must have been that the two "ANY"
signatures should change to "missing".

    > Definitely.

    >> However, with the current logic that will not happen, because
    >> 
    >> > c(F,T,T,F) &&  c(T,T)
    >> [1] FALSE
    >> 
    >> Henrik's non-fix would have resulted in
    >> 
    >> > c(F,T,T,F) &  c(T,T)
    >> [1] FALSE  TRUE  TRUE FALSE
    >> 
    >> which is actually right, but only coincidentally due to recycling
of c(T,T). Had it been c(F,T) then it would have been expanded to c(F,T,F,T)
which would be the opposite of what was wanted.
    >> 
    >> Barring NA issues, I still think 
    >> 
    >> omittedSig[omittedSig] <- (signature[omittedSig] !=
"missing")
    >> 
    >> should do the trick.

    > yes, (most probably).  I've found a version of that which should
    > be even easier to "read and understand", in  svn commit 76753
:

    > svn diff -c 76753 src/library/methods/R/RMethodUtils.R

    > --- src/library/methods/R/RMethodUtils.R	(Revision 76752)
    > +++ src/library/methods/R/RMethodUtils.R	(Revision 76753)
    > @@ -342,8 +342,7 @@
    > gettextf("formal arguments (%s) omitted in the method definition
cannot be in the signature", bad2),
    > call. = TRUE, domain = NA)
    > }
    > -    else if(!all(signature[omittedSig] == "missing")) {
    > -        omittedSig <- omittedSig && (signature[omittedSig]
!= "missing")
    > +    else if(any(omittedSig <- omittedSig & signature !=
"missing")) {


    > BTW:  I've marked this --- and the  runmed() seg.fault + na.action
    > change ---  as something to be added to R 3.6.1 patched,  as I
    > deemed I should obey the "code freeze" rule in both cases.

    > Martin

Hmm... I think we got a 'Neverending Story' here -- because it
seems, both Peter and I were wrong in thinking that it's a good
idea to change "missing" to "ANY" here ...
((or if that *was* correct, that needs to entail more changes
happening during setMethod(.) {conformMethod() is only called in
one place in our code base, namely from setMethod()} :

I've started to explore the effects of the change using and
extending the tests/reg-tests-1d.R  example that I had just committed.

And the result is *not good* :

As we said above, the new code does replace all "ANY" by
"missing" in the
signature, unless the "ANY" are at the end of the signature.

However, later methods package code producing the .local(.)
calls in the method definition (in cases where the signature of
the generic and the method do not match exactly) --- possibly
may have been tweaked later than the conformMethod() code ---
and the .local() calls they now produce

- work as intended for R <= 3.6.0 (and R 3.6.1 RC)
- fail to work for R-devel with the change :

See this (not a minimal rep.rex. but one building on Henrik's
          oligo case and what's newly in  tests/reg-tests-1d.R ) :

-----------------------------------------------------------------------------

## conformMethod()  "&& logic" bug, by Henrik Bengtsson on
R-devel list, 2019-06-22
setClass("tilingFSet", slots = c(x = "numeric"))
if(!is.null(getGeneric("oligoFn"))) removeGeneric("oligoFn")
setGeneric("oligoFn",
           function(object, subset, target, value) {
standardGeneric("oligoFn") })
Sys.unsetenv("_R_CHECK_LENGTH_1_LOGIC2_") ## << added here, to
compare with R 3.6.0, 3.5.3, ..
setMethod("oligoFn", signature(object = "tilingFSet",
value="array"),	## Method _ 1 _
          function(object, value) { list(object=object, value=value) })
setMethod("oligoFn", signature(object = "matrix",
target="array"),	## Method _ 2 _
          function(object, target) list(object=object, target=target))
setMethod("oligoFn", signature(object = "matrix",
subset="integer"),	## Method _ 3 _
          function(object, subset) list(object=object, subset=subset))	#	(*no*
Note: ANY at end)
setMethod("oligoFn", signature(object = "matrix"),			##
Method _ 4 _
          function(object) list(object=object))				#	(*no* Note: ANY at end)
showMethods("oligoFn") # F.Y.I.:  in R 3.6.0 and earlier: contains
"ANY" everywhere

##-- Now, the following *DOES* work fine in R <= 3.6.0  but *no longer* in
R-devel :
str(r1 <- oligoFn(object=new("tilingFSet"), value=array(2)))
str(r2 <- oligoFn(object=diag(2),          target=array(42)))
## These 2 work fine in all versions of R: Here the "ANY" remain at
the end:
str(r3 <- oligoFn(object=diag(2),          subset=1:3))
str(r4 <- oligoFn(object=diag(2)))

-----------------------------------------------------------------------------
The two errors in R-devel are actually quite user-confusing:
> r2 <- oligoFn(object=diag(2),          target=array(42))
Error in .local(object, target) : 
  argument "target" is missing, with no
default
> getMethod("oligoFn", signature(object="matrix",
subset="missing", target="array"))
Method Definition:

function (object, subset, target, value) 
{
    .local <- function (object, subset, target, value) 
    list(object = object, target = target)
    .local(object, target)
}

Signatures:
        object   subset    target  value
target  "matrix" "missing" "array" "ANY"
defined "matrix" "missing" "array"
"ANY"
> 

My conclusion:  There's something really wrong with what
conformMethod() has been *intended* to achieve and what it
"accidentally" did achieve in these cases: it never replaced
"ANY" by "missing" in all these cases *AND* that is what it
seems it should continue doing.

BTW: ?conformMethod  goes to a page with quite a few things,
     relevantly containing

  ?conformMethod?: If the formal arguments, ?mnames?, are not
       identical to the formal arguments to the function, ?fnames?,
       ?conformMethod? determines whether the signature and the two
       sets of arguments conform, and returns the signature,
       possibly extended.  The function name, ?f? is supplied for
       error messages. The generic function, ?fdef?, supplies the
       generic signature for matching purposes.

       The method assignment conforms if method and generic function
       have identical formal argument lists.  It can also conform if
       the method omits some of the formal arguments of the function
       but: (1) the non-omitted arguments are a subset of the
       function arguments, appearing in the same order; (2) there
       are no arguments to the method that are not arguments to the
       function; and (3) the omitted formal arguments do not appear
       as explicit classes in the signature.  A future extension
       hopes to test also that the omitted arguments are not assumed
       by being used as locally assigned names or function names in
       the body of the method.

---
It seems my commit to R-devel needs another change.
This has to wait for hours currently, though.

Martin

>>>>> Martin Maechler 
>>>>>     on Sat, 29 Jun 2019 12:05:49 +0200 writes:
>>>>> Martin Maechler 
>>>>>     on Sat, 29 Jun 2019 10:33:10 +0200 writes:
>>>>> peter dalgaard 
>>>>>     on Fri, 28 Jun 2019 16:20:03 +0200 writes:
    >>> > On 28 Jun 2019, at 16:03 , Martin Maechler <maechler at
stat.math.ethz.ch> wrote:
    >>> > 
    >>> >>>>>> Henrik Bengtsson 
    >>> >>>>>>    on Thu, 27 Jun 2019 16:00:39 -0700
writes:
    >>> > 
    >>> >> Using:
    >>> >> 
    >>> >> untrace(methods::conformMethod)
    >>> >> at <- c(12,4,3,2)
    >>> >> str(body(methods::conformMethod)[[at]])
    >>> >> ## language omittedSig <- omittedSig &&
(signature[omittedSig] != "missing")
    >>> >> cc <- 0L
    >>> >> trace(methods::conformMethod, tracer = quote({
    >>> >>  cc <<- cc + 1L
    >>> >>  print(cc)
    >>> >>  if (cc == 31) {  ## manually identified
    >>> >>    untrace(methods::conformMethod)
    >>> >>    trace(methods::conformMethod, at = list(at), tracer
= quote({
    >>> >>      str(list(signature = signature, mnames = mnames,
fnames = fnames))
    >>> >>      print(ls())
    >>> >>      try(str(list(omittedSig = omittedSig, signature =
signature)))
    >>> >>    }))
    >>> >>  }
    >>> >> }))
    >>> >> loadNamespace("oligo")
    >>> >> 
    >>> >> gives:
    >>> >> 
    >>> >> Untracing function "conformMethod" in
package "methods"
    >>> >> Tracing function "conformMethod" in package
"methods"
    >>> >> Tracing conformMethod(signature, mnames, fnames, f,
fdef, definition)
    >>> >> step 12,4,3,2
    >>> >> List of 3
    >>> >> $ signature: Named chr [1:4]
"TilingFeatureSet" "ANY" "ANY" "array"
    >>> >>  ..- attr(*, "names")= chr [1:4]
"object" "subset" "target" "value"
    >>> >>  ..- attr(*, "package")= chr [1:4]
"oligoClasses" "methods" "methods"
"methods"
    >>> >> $ mnames   : chr [1:2] "object"
"value"
    >>> >> $ fnames   : chr [1:4] "object"
"subset" "target" "value"
    >>> >> [1] "f"          "fdef"      
"fnames"     "fsig"       "imf"
    >>> >> [6] "method"     "mnames"    
"omitted"    "omittedSig" "sig0"
    >>> >> [11] "sigNames"   "signature"
    >>> >> List of 2
    >>> >> $ omittedSig: logi [1:4] FALSE TRUE TRUE FALSE
    >>> >> $ signature : Named chr [1:4]
"TilingFeatureSet" "ANY" "ANY" "array"
    >>> >>  ..- attr(*, "names")= chr [1:4]
"object" "subset" "target" "value"
    >>> >>  ..- attr(*, "package")= chr [1:4]
"oligoClasses" "methods" "methods"
"methods"
    >>> >> Error in omittedSig && (signature[omittedSig]
!= "missing") :
    >>> >>  'length(x) = 4 > 1' in coercion to
'logical(1)'
    >>> >> Error: unable to load R code in package
'oligo'
    >>> >> 
    >>> > 
    >>> > Thank you, Henrik, nice piece of using trace() .. and the
above
    >>> > is useful for solving the issue --  I can work with that.
    >>> > 
    >>> > I'm  already pretty sure the wrong code starts with
    >>> > 
    >>> >    omittedSig <- sigNames %in% fnames[omitted] # ....

    >> my  "pretty sure"  statement above has proven to be wrong
..

    >>> > -------------
    >>> > 
    >>> 
    >>> I think the intention must have been that the two
"ANY" signatures should change to "missing".

    >> Definitely.

    >>> However, with the current logic that will not happen, because
    >>> 
    >>> > c(F,T,T,F) &&  c(T,T)
    >>> [1] FALSE
    >>> 
    >>> Henrik's non-fix would have resulted in
    >>> 
    >>> > c(F,T,T,F) &  c(T,T)
    >>> [1] FALSE  TRUE  TRUE FALSE
    >>> 
    >>> which is actually right, but only coincidentally due to
recycling of c(T,T). Had it been c(F,T) then it would have been expanded to
c(F,T,F,T) which would be the opposite of what was wanted.
    >>> 
    >>> Barring NA issues, I still think 
    >>> 
    >>> omittedSig[omittedSig] <- (signature[omittedSig] !=
"missing")
    >>> 
    >>> should do the trick.

    >> yes, (most probably).  I've found a version of that which
should
    >> be even easier to "read and understand", in  svn commit
76753 :

    >> svn diff -c 76753 src/library/methods/R/RMethodUtils.R

    >> --- src/library/methods/R/RMethodUtils.R	(Revision 76752)
    >> +++ src/library/methods/R/RMethodUtils.R	(Revision 76753)
    >> @@ -342,8 +342,7 @@
    >> gettextf("formal arguments (%s) omitted in the method
definition cannot be in the signature", bad2),
    >> call. = TRUE, domain = NA)
    >> }
    >> -    else if(!all(signature[omittedSig] == "missing")) {
    >> -        omittedSig <- omittedSig &&
(signature[omittedSig] != "missing")
    >> +    else if(any(omittedSig <- omittedSig & signature !=
"missing")) {


    >> BTW:  I've marked this --- and the  runmed() seg.fault +
na.action
    >> change ---  as something to be added to R 3.6.1 patched,  as I
    >> deemed I should obey the "code freeze" rule in both
cases.

    >> Martin

    > Hmm... I think we got a 'Neverending Story' here -- because it
    > seems, both Peter and I were wrong in thinking that it's a good
    > idea to change "missing" to "ANY" here ...
    > ((or if that *was* correct, that needs to entail more changes
    > happening during setMethod(.) {conformMethod() is only called in
    > one place in our code base, namely from setMethod()} :

as a matter of fact, I've been brave for now, left the change to
conformMethod()  and started to fix the constructed .local()
calls which are created in  conformMethod()'s sibling,
which is rematchDefinition().

It seems that this builds e.g. Matrix {with its gazillion
setMethod()s} and that continues to run its own tests.  OTOH,
Matrix may not trigger the situations that are dealt with here
at all, as the signature() are rarely longer than three, and at
some point in time I had made long passes through the package in
order to "minimize" the .local() calls.

--> svn commit 76756  (in addition to 76753, mentioned earlier)
    now has rematchDefinition() changes

I would be positively surprised if (but can imagine that) this
had no affect on CRAN / Bioconductor packages.

Still, these two changes seem to achieve what both the comments
and the documentation of  conformMethod() and rematchDefinition()
suggest should happen.

Of course, I'd really be happy if people with S4 packages would
check them with an R-devel version with svn rev >=  76756
and report problems they see.
I do imagine effects, and would expect that bugs in current code
become visible where they had not done so previously.

Martin

    > I've started to explore the effects of the change using and
    > extending the tests/reg-tests-1d.R  example that I had just committed.

    > And the result is *not good* :

    > As we said above, the new code does replace all "ANY" by
"missing" in the
    > signature, unless the "ANY" are at the end of the signature.

    > However, later methods package code producing the .local(.)
    > calls in the method definition (in cases where the signature of
    > the generic and the method do not match exactly) --- possibly
    > may have been tweaked later than the conformMethod() code ---
    > and the .local() calls they now produce

    > - work as intended for R <= 3.6.0 (and R 3.6.1 RC)
    > - fail to work for R-devel with the change :

    > See this (not a minimal rep.rex. but one building on Henrik's
    > oligo case and what's newly in  tests/reg-tests-1d.R ) :

    >
-----------------------------------------------------------------------------

    > ## conformMethod()  "&& logic" bug, by Henrik
Bengtsson on R-devel list, 2019-06-22
    > setClass("tilingFSet", slots = c(x = "numeric"))
    > if(!is.null(getGeneric("oligoFn")))
removeGeneric("oligoFn")
    > setGeneric("oligoFn",
    > function(object, subset, target, value) {
standardGeneric("oligoFn") })
    > Sys.unsetenv("_R_CHECK_LENGTH_1_LOGIC2_") ## << added
here, to compare with R 3.6.0, 3.5.3, ..
    > setMethod("oligoFn", signature(object =
"tilingFSet", value="array"),	## Method _ 1 _
    > function(object, value) { list(object=object, value=value) })
    > setMethod("oligoFn", signature(object = "matrix",
target="array"),	## Method _ 2 _
    > function(object, target) list(object=object, target=target))
    > setMethod("oligoFn", signature(object = "matrix",
subset="integer"),	## Method _ 3 _
    > function(object, subset) list(object=object, subset=subset))	#	(*no*
Note: ANY at end)
    > setMethod("oligoFn", signature(object = "matrix"),	
## Method _ 4 _
    > function(object) list(object=object))				#	(*no* Note: ANY at end)
    > showMethods("oligoFn") # F.Y.I.:  in R 3.6.0 and earlier:
contains "ANY" everywhere

    > ##-- Now, the following *DOES* work fine in R <= 3.6.0  but *no
longer* in R-devel :
    > str(r1 <- oligoFn(object=new("tilingFSet"),
value=array(2)))
    > str(r2 <- oligoFn(object=diag(2),          target=array(42)))
    > ## These 2 work fine in all versions of R: Here the "ANY"
remain at the end:
    > str(r3 <- oligoFn(object=diag(2),          subset=1:3))
    > str(r4 <- oligoFn(object=diag(2)))

    >
-----------------------------------------------------------------------------
    > The two errors in R-devel are actually quite user-confusing:

    >> r2 <- oligoFn(object=diag(2),          target=array(42))
    > Error in .local(object, target) : 
    > argument "target" is missing, with no default
    >> getMethod("oligoFn", signature(object="matrix",
subset="missing", target="array"))
    > Method Definition:

    > function (object, subset, target, value) 
    > {
    > .local <- function (object, subset, target, value) 
    > list(object = object, target = target)
    > .local(object, target)
    > }

    > Signatures:
    > object   subset    target  value
    > target  "matrix" "missing" "array"
"ANY"
    > defined "matrix" "missing" "array"
"ANY"
    >> 


    > My conclusion:  There's something really wrong with what
    > conformMethod() has been *intended* to achieve and what it
    > "accidentally" did achieve in these cases: it never replaced
    > "ANY" by "missing" in all these cases *AND* that is
what it
    > seems it should continue doing.

    > BTW: ?conformMethod  goes to a page with quite a few things,
    > relevantly containing

    > ?conformMethod?: If the formal arguments, ?mnames?, are not
    > identical to the formal arguments to the function, ?fnames?,
    > ?conformMethod? determines whether the signature and the two
    > sets of arguments conform, and returns the signature,
    > possibly extended.  The function name, ?f? is supplied for
    > error messages. The generic function, ?fdef?, supplies the
    > generic signature for matching purposes.

    > The method assignment conforms if method and generic function
    > have identical formal argument lists.  It can also conform if
    > the method omits some of the formal arguments of the function
    > but: (1) the non-omitted arguments are a subset of the
    > function arguments, appearing in the same order; (2) there
    > are no arguments to the method that are not arguments to the
    > function; and (3) the omitted formal arguments do not appear
    > as explicit classes in the signature.  A future extension
    > hopes to test also that the omitted arguments are not assumed
    > by being used as locally assigned names or function names in
    > the body of the method.

    > ---
    > It seems my commit to R-devel needs another change.
    > This has to wait for hours currently, though.

    > Martin

    > ______________________________________________
    > R-devel at r-project.org mailing list
    > https://stat.ethz.ch/mailman/listinfo/r-devel

I still observe this error and just want to ping this thread so we
don't forget it. Should I add this to
https://bugs.r-project.org/bugzilla/ so it's tracked there?

This thread in the archives:

* https://stat.ethz.ch/pipermail/r-devel/2019-June/078049.html
* https://stat.ethz.ch/pipermail/r-devel/2019-July/078115.html
* https://stat.ethz.ch/pipermail/r-devel/2019-July/078126.html

/Henrik

On Sat, Jun 29, 2019 at 1:45 PM Martin Maechler
<maechler at stat.math.ethz.ch> wrote:
>
> >>>>> Martin Maechler
> >>>>>     on Sat, 29 Jun 2019 12:05:49 +0200 writes:
>
> >>>>> Martin Maechler
> >>>>>     on Sat, 29 Jun 2019 10:33:10 +0200 writes:
>
> >>>>> peter dalgaard
> >>>>>     on Fri, 28 Jun 2019 16:20:03 +0200 writes:
>
>     >>> > On 28 Jun 2019, at 16:03 , Martin Maechler
<maechler at stat.math.ethz.ch> wrote:
>     >>> >
>     >>> >>>>>> Henrik Bengtsson
>     >>> >>>>>>    on Thu, 27 Jun 2019 16:00:39
-0700 writes:
>     >>> >
>     >>> >> Using:
>     >>> >>
>     >>> >> untrace(methods::conformMethod)
>     >>> >> at <- c(12,4,3,2)
>     >>> >> str(body(methods::conformMethod)[[at]])
>     >>> >> ## language omittedSig <- omittedSig
&& (signature[omittedSig] != "missing")
>     >>> >> cc <- 0L
>     >>> >> trace(methods::conformMethod, tracer = quote({
>     >>> >>  cc <<- cc + 1L
>     >>> >>  print(cc)
>     >>> >>  if (cc == 31) {  ## manually identified
>     >>> >>    untrace(methods::conformMethod)
>     >>> >>    trace(methods::conformMethod, at = list(at),
tracer = quote({
>     >>> >>      str(list(signature = signature, mnames =
mnames, fnames = fnames))
>     >>> >>      print(ls())
>     >>> >>      try(str(list(omittedSig = omittedSig,
signature = signature)))
>     >>> >>    }))
>     >>> >>  }
>     >>> >> }))
>     >>> >> loadNamespace("oligo")
>     >>> >>
>     >>> >> gives:
>     >>> >>
>     >>> >> Untracing function "conformMethod" in
package "methods"
>     >>> >> Tracing function "conformMethod" in
package "methods"
>     >>> >> Tracing conformMethod(signature, mnames, fnames,
f, fdef, definition)
>     >>> >> step 12,4,3,2
>     >>> >> List of 3
>     >>> >> $ signature: Named chr [1:4]
"TilingFeatureSet" "ANY" "ANY" "array"
>     >>> >>  ..- attr(*, "names")= chr [1:4]
"object" "subset" "target" "value"
>     >>> >>  ..- attr(*, "package")= chr [1:4]
"oligoClasses" "methods" "methods"
"methods"
>     >>> >> $ mnames   : chr [1:2] "object"
"value"
>     >>> >> $ fnames   : chr [1:4] "object"
"subset" "target" "value"
>     >>> >> [1] "f"          "fdef"      
"fnames"     "fsig"       "imf"
>     >>> >> [6] "method"     "mnames"    
"omitted"    "omittedSig" "sig0"
>     >>> >> [11] "sigNames"   "signature"
>     >>> >> List of 2
>     >>> >> $ omittedSig: logi [1:4] FALSE TRUE TRUE FALSE
>     >>> >> $ signature : Named chr [1:4]
"TilingFeatureSet" "ANY" "ANY" "array"
>     >>> >>  ..- attr(*, "names")= chr [1:4]
"object" "subset" "target" "value"
>     >>> >>  ..- attr(*, "package")= chr [1:4]
"oligoClasses" "methods" "methods"
"methods"
>     >>> >> Error in omittedSig &&
(signature[omittedSig] != "missing") :
>     >>> >>  'length(x) = 4 > 1' in coercion to
'logical(1)'
>     >>> >> Error: unable to load R code in package
'oligo'
>     >>> >>
>     >>> >
>     >>> > Thank you, Henrik, nice piece of using trace() .. and
the above
>     >>> > is useful for solving the issue --  I can work with
that.
>     >>> >
>     >>> > I'm  already pretty sure the wrong code starts
with
>     >>> >
>     >>> >    omittedSig <- sigNames %in% fnames[omitted] #
....
>
>     >> my  "pretty sure"  statement above has proven to be
wrong ..
>
>     >>> > -------------
>     >>> >
>     >>>
>     >>> I think the intention must have been that the two
"ANY" signatures should change to "missing".
>
>     >> Definitely.
>
>     >>> However, with the current logic that will not happen,
because
>     >>>
>     >>> > c(F,T,T,F) &&  c(T,T)
>     >>> [1] FALSE
>     >>>
>     >>> Henrik's non-fix would have resulted in
>     >>>
>     >>> > c(F,T,T,F) &  c(T,T)
>     >>> [1] FALSE  TRUE  TRUE FALSE
>     >>>
>     >>> which is actually right, but only coincidentally due to
recycling of c(T,T). Had it been c(F,T) then it would have been expanded to
c(F,T,F,T) which would be the opposite of what was wanted.
>     >>>
>     >>> Barring NA issues, I still think
>     >>>
>     >>> omittedSig[omittedSig] <- (signature[omittedSig] !=
"missing")
>     >>>
>     >>> should do the trick.
>
>     >> yes, (most probably).  I've found a version of that which
should
>     >> be even easier to "read and understand", in  svn
commit 76753 :
>
>     >> svn diff -c 76753 src/library/methods/R/RMethodUtils.R
>
>     >> --- src/library/methods/R/RMethodUtils.R (Revision 76752)
>     >> +++ src/library/methods/R/RMethodUtils.R (Revision 76753)
>     >> @@ -342,8 +342,7 @@
>     >> gettextf("formal arguments (%s) omitted in the method
definition cannot be in the signature", bad2),
>     >> call. = TRUE, domain = NA)
>     >> }
>     >> -    else if(!all(signature[omittedSig] ==
"missing")) {
>     >> -        omittedSig <- omittedSig &&
(signature[omittedSig] != "missing")
>     >> +    else if(any(omittedSig <- omittedSig & signature
!= "missing")) {
>
>
>     >> BTW:  I've marked this --- and the  runmed() seg.fault +
na.action
>     >> change ---  as something to be added to R 3.6.1 patched,  as I
>     >> deemed I should obey the "code freeze" rule in both
cases.
>
>     >> Martin
>
>     > Hmm... I think we got a 'Neverending Story' here --
because it
>     > seems, both Peter and I were wrong in thinking that it's a
good
>     > idea to change "missing" to "ANY" here ...
>     > ((or if that *was* correct, that needs to entail more changes
>     > happening during setMethod(.) {conformMethod() is only called in
>     > one place in our code base, namely from setMethod()} :
>
> as a matter of fact, I've been brave for now, left the change to
> conformMethod()  and started to fix the constructed .local()
> calls which are created in  conformMethod()'s sibling,
> which is rematchDefinition().
>
> It seems that this builds e.g. Matrix {with its gazillion
> setMethod()s} and that continues to run its own tests.  OTOH,
> Matrix may not trigger the situations that are dealt with here
> at all, as the signature() are rarely longer than three, and at
> some point in time I had made long passes through the package in
> order to "minimize" the .local() calls.
>
> --> svn commit 76756  (in addition to 76753, mentioned earlier)
>     now has rematchDefinition() changes
>
> I would be positively surprised if (but can imagine that) this
> had no affect on CRAN / Bioconductor packages.
>
> Still, these two changes seem to achieve what both the comments
> and the documentation of  conformMethod() and rematchDefinition()
> suggest should happen.
>
> Of course, I'd really be happy if people with S4 packages would
> check them with an R-devel version with svn rev >=  76756
> and report problems they see.
> I do imagine effects, and would expect that bugs in current code
> become visible where they had not done so previously.
>
> Martin
>
>     > I've started to explore the effects of the change using and
>     > extending the tests/reg-tests-1d.R  example that I had just
committed.
>
>     > And the result is *not good* :
>
>     > As we said above, the new code does replace all "ANY" by
"missing" in the
>     > signature, unless the "ANY" are at the end of the
signature.
>
>     > However, later methods package code producing the .local(.)
>     > calls in the method definition (in cases where the signature of
>     > the generic and the method do not match exactly) --- possibly
>     > may have been tweaked later than the conformMethod() code ---
>     > and the .local() calls they now produce
>
>     > - work as intended for R <= 3.6.0 (and R 3.6.1 RC)
>     > - fail to work for R-devel with the change :
>
>     > See this (not a minimal rep.rex. but one building on Henrik's
>     > oligo case and what's newly in  tests/reg-tests-1d.R ) :
>
>     >
-----------------------------------------------------------------------------
>
>     > ## conformMethod()  "&& logic" bug, by Henrik
Bengtsson on R-devel list, 2019-06-22
>     > setClass("tilingFSet", slots = c(x =
"numeric"))
>     > if(!is.null(getGeneric("oligoFn")))
removeGeneric("oligoFn")
>     > setGeneric("oligoFn",
>     > function(object, subset, target, value) {
standardGeneric("oligoFn") })
>     > Sys.unsetenv("_R_CHECK_LENGTH_1_LOGIC2_") ## <<
added here, to compare with R 3.6.0, 3.5.3, ..
>     > setMethod("oligoFn", signature(object =
"tilingFSet", value="array"),     ## Method _ 1 _
>     > function(object, value) { list(object=object, value=value) })
>     > setMethod("oligoFn", signature(object =
"matrix", target="array"),        ## Method _ 2 _
>     > function(object, target) list(object=object, target=target))
>     > setMethod("oligoFn", signature(object =
"matrix", subset="integer"),      ## Method _ 3 _
>     > function(object, subset) list(object=object, subset=subset))     
#       (*no* Note: ANY at end)
>     > setMethod("oligoFn", signature(object =
"matrix"),                        ## Method _ 4 _
>     > function(object) list(object=object))                            
#       (*no* Note: ANY at end)
>     > showMethods("oligoFn") # F.Y.I.:  in R 3.6.0 and
earlier: contains "ANY" everywhere
>
>     > ##-- Now, the following *DOES* work fine in R <= 3.6.0  but *no
longer* in R-devel :
>     > str(r1 <- oligoFn(object=new("tilingFSet"),
value=array(2)))
>     > str(r2 <- oligoFn(object=diag(2),          target=array(42)))
>     > ## These 2 work fine in all versions of R: Here the
"ANY" remain at the end:
>     > str(r3 <- oligoFn(object=diag(2),          subset=1:3))
>     > str(r4 <- oligoFn(object=diag(2)))
>
>     >
-----------------------------------------------------------------------------
>     > The two errors in R-devel are actually quite user-confusing:
>
>     >> r2 <- oligoFn(object=diag(2),          target=array(42))
>     > Error in .local(object, target) :
>     > argument "target" is missing, with no default
>     >> getMethod("oligoFn",
signature(object="matrix", subset="missing",
target="array"))
>     > Method Definition:
>
>     > function (object, subset, target, value)
>     > {
>     > .local <- function (object, subset, target, value)
>     > list(object = object, target = target)
>     > .local(object, target)
>     > }
>
>     > Signatures:
>     > object   subset    target  value
>     > target  "matrix" "missing" "array"
"ANY"
>     > defined "matrix" "missing" "array"
"ANY"
>     >>
>
>
>     > My conclusion:  There's something really wrong with what
>     > conformMethod() has been *intended* to achieve and what it
>     > "accidentally" did achieve in these cases: it never
replaced
>     > "ANY" by "missing" in all these cases *AND*
that is what it
>     > seems it should continue doing.
>
>     > BTW: ?conformMethod  goes to a page with quite a few things,
>     > relevantly containing
>
>     > ?conformMethod?: If the formal arguments, ?mnames?, are not
>     > identical to the formal arguments to the function, ?fnames?,
>     > ?conformMethod? determines whether the signature and the two
>     > sets of arguments conform, and returns the signature,
>     > possibly extended.  The function name, ?f? is supplied for
>     > error messages. The generic function, ?fdef?, supplies the
>     > generic signature for matching purposes.
>
>     > The method assignment conforms if method and generic function
>     > have identical formal argument lists.  It can also conform if
>     > the method omits some of the formal arguments of the function
>     > but: (1) the non-omitted arguments are a subset of the
>     > function arguments, appearing in the same order; (2) there
>     > are no arguments to the method that are not arguments to the
>     > function; and (3) the omitted formal arguments do not appear
>     > as explicit classes in the signature.  A future extension
>     > hopes to test also that the omitted arguments are not assumed
>     > by being used as locally assigned names or function names in
>     > the body of the method.
>
>     > ---
>     > It seems my commit to R-devel needs another change.
>     > This has to wait for hours currently, though.
>
>     > Martin
>
>     > ______________________________________________
>     > R-devel at r-project.org mailing list
>     > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel