methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Henrik Bengtsson-5
DISCLAIMER: I can not get this error with R --vanilla, so it only
occurs when some other package is also loaded.  I don't have time to
find to narrow that down for a reproducible example, but I believe the
following error in R 3.6.0:

> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
> library(oligo)
Error in omittedSig && (signature[omittedSig] != "missing") :
  'length(x) = 4 > 1' in coercion to 'logical(1)'
Error: unable to load R code in package 'oligo'

is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
'methods' package.  Here's the patch:

$ svn diff src/library/methods/R/RMethodUtils.R &
[1] 1062
Index: src/library/methods/R/RMethodUtils.R
===================================================================
--- src/library/methods/R/RMethodUtils.R (revision 76731)
+++ src/library/methods/R/RMethodUtils.R (working copy)
@@ -343,7 +343,7 @@
              call. = TRUE, domain = NA)
     }
     else if(!all(signature[omittedSig] == "missing")) {
-        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
+        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
         .message("Note: ", .renderSignature(f, sig0),
                  gettextf("expanding the signature to include omitted
arguments in definition: %s",
                           paste(sigNames[omittedSig], "=
\"missing\"",collapse = ", ")))
[1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R

Maybe still in time for R 3.6.1?

/Henrik

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Peter Dalgaard-2
This looks obvious enough, so I just committed your fix to R-devel and R-patched.

I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.

-pd

> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
>
> DISCLAIMER: I can not get this error with R --vanilla, so it only
> occurs when some other package is also loaded.  I don't have time to
> find to narrow that down for a reproducible example, but I believe the
> following error in R 3.6.0:
>
>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>> library(oligo)
> Error in omittedSig && (signature[omittedSig] != "missing") :
>  'length(x) = 4 > 1' in coercion to 'logical(1)'
> Error: unable to load R code in package 'oligo'
>
> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
> 'methods' package.  Here's the patch:
>
> $ svn diff src/library/methods/R/RMethodUtils.R &
> [1] 1062
> Index: src/library/methods/R/RMethodUtils.R
> ===================================================================
> --- src/library/methods/R/RMethodUtils.R (revision 76731)
> +++ src/library/methods/R/RMethodUtils.R (working copy)
> @@ -343,7 +343,7 @@
>              call. = TRUE, domain = NA)
>     }
>     else if(!all(signature[omittedSig] == "missing")) {
> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
> +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>         .message("Note: ", .renderSignature(f, sig0),
>                  gettextf("expanding the signature to include omitted
> arguments in definition: %s",
>                           paste(sigNames[omittedSig], "=
> \"missing\"",collapse = ", ")))
> [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>
> Maybe still in time for R 3.6.1?
>
> /Henrik
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Henrik Bengtsson-5
Thank you.

To correct myself, I can indeed reproduce this with R --vanilla too.
A reproducible example is:

$ R --vanilla
R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
...
> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
> loadNamespace("oligo")
Error in omittedSig && (signature[omittedSig] != "missing") :
  'length(x) = 4 > 1' in coercion to 'logical(1)'
Error: unable to load R code in package ‘oligo’

/Henrik

On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:

>
> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>
> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>
> -pd
>
> > On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
> >
> > DISCLAIMER: I can not get this error with R --vanilla, so it only
> > occurs when some other package is also loaded.  I don't have time to
> > find to narrow that down for a reproducible example, but I believe the
> > following error in R 3.6.0:
> >
> >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
> >> library(oligo)
> > Error in omittedSig && (signature[omittedSig] != "missing") :
> >  'length(x) = 4 > 1' in coercion to 'logical(1)'
> > Error: unable to load R code in package 'oligo'
> >
> > is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
> > 'methods' package.  Here's the patch:
> >
> > $ svn diff src/library/methods/R/RMethodUtils.R &
> > [1] 1062
> > Index: src/library/methods/R/RMethodUtils.R
> > ===================================================================
> > --- src/library/methods/R/RMethodUtils.R (revision 76731)
> > +++ src/library/methods/R/RMethodUtils.R (working copy)
> > @@ -343,7 +343,7 @@
> >              call. = TRUE, domain = NA)
> >     }
> >     else if(!all(signature[omittedSig] == "missing")) {
> > -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
> > +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
> >         .message("Note: ", .renderSignature(f, sig0),
> >                  gettextf("expanding the signature to include omitted
> > arguments in definition: %s",
> >                           paste(sigNames[omittedSig], "=
> > \"missing\"",collapse = ", ")))
> > [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
> >
> > Maybe still in time for R 3.6.1?
> >
> > /Henrik
> >
> > ______________________________________________
> > [hidden email] mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: [hidden email]  Priv: [hidden email]
>
>
>
>
>
>
>
>
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> Henrik Bengtsson via R-core
>>>>>     on Sun, 23 Jun 2019 11:29:58 -0700 writes:

    > Thank you.
    > To correct myself, I can indeed reproduce this with R --vanilla too.
    > A reproducible example is:

    > $ R --vanilla
    > R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
    > ...
    >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
    >> loadNamespace("oligo")
    > Error in omittedSig && (signature[omittedSig] != "missing") :
    > 'length(x) = 4 > 1' in coercion to 'logical(1)'
    > Error: unable to load R code in package ‘oligo’

    > /Henrik

Thank you Henrik, for the report, etc, but
hmm... after loading the oligo package, almost 40 (non
base+Recommended) packages have been loaded as well, which hence
need to have been installed before, too ..
which is not quite a "vanilla repr.ex." in my view

Worse, I cannot reproduce :

    > Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
    > Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
    [1] "true"
    > debugonce(conformMethod)
    > loadNamespace("oligo")
    <environment: namespace:oligo>
    Warning messages:
    1: multiple methods tables found for ‘rowSums’
    2: multiple methods tables found for ‘colSums’
    3: multiple methods tables found for ‘rowMeans’
    4: multiple methods tables found for ‘colMeans’
    > sessionInfo()
    R Under development (unstable) (2019-06-20 r76729)

(similarly with other versions of R >= 3.6.0).

So, even though R core has fixed this now in the sources, it
would be nice to have an "as simple as possible"  repr.ex. for this.

Martin



    > On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
    >>
    >> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
    >>
    >> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
    >>
    >> -pd
    >>
    >> > On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
    >> >
    >> > DISCLAIMER: I can not get this error with R --vanilla, so it only
    >> > occurs when some other package is also loaded.  I don't have time to
    >> > find to narrow that down for a reproducible example, but I believe the
    >> > following error in R 3.6.0:
    >> >
    >> >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
    >> >> library(oligo)
    >> > Error in omittedSig && (signature[omittedSig] != "missing") :
    >> >  'length(x) = 4 > 1' in coercion to 'logical(1)'
    >> > Error: unable to load R code in package 'oligo'
    >> >
    >> > is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
    >> > 'methods' package.  Here's the patch:
    >> >
    >> > $ svn diff src/library/methods/R/RMethodUtils.R &
    >> > [1] 1062
    >> > Index: src/library/methods/R/RMethodUtils.R
    >> > ===================================================================
    >> > --- src/library/methods/R/RMethodUtils.R (revision 76731)
    >> > +++ src/library/methods/R/RMethodUtils.R (working copy)
    >> > @@ -343,7 +343,7 @@
    >> >              call. = TRUE, domain = NA)
    >> >     }
    >> >     else if(!all(signature[omittedSig] == "missing")) {
    >> > -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
    >> > +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
    >> >         .message("Note: ", .renderSignature(f, sig0),
    >> >                  gettextf("expanding the signature to include omitted
    >> > arguments in definition: %s",
    >> >                           paste(sigNames[omittedSig], "=
    >> > \"missing\"",collapse = ", ")))
    >> > [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
    >> >
    >> > Maybe still in time for R 3.6.1?
    >> >
    >> > /Henrik
    >> >
    >> > ______________________________________________
    >> > [hidden email] mailing list
    >> > https://stat.ethz.ch/mailman/listinfo/r-devel
    >>
    >> --
    >> Peter Dalgaard, Professor,
    >> Center for Statistics, Copenhagen Business School
    >> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
    >> Phone: (+45)38153501
    >> Office: A 4.23
    >> Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Henrik Bengtsson-5
**Maybe this bug needs to be understood further before applying the
patch because patch is most likely also wrong**

Because, from just looking at the expressions, I think neither the R
3.6.0 version:

omittedSig <- omittedSig && (signature[omittedSig] != "missing")

nor the patched version (I proposed):

omittedSig <- omittedSig & (signature[omittedSig] != "missing")

can be correct.  For a starter, 'omittedSig' is a logical vector.  We
see that 'omittedSig' is used to subset 'signature'.  In other words,
the length of 'signature[omittedSig]' and hence the length of
'(signature[omittedSig] != "missing")' will equal sum(omittedSig),
i.e. the length will be in {0,1,...,length(omittedSig)}.

The R 3.6.0 version with '&&' is not correct because '&&' requires
length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
to be the original intention.

The patched version with '&' is most likely not correct either because
'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
auto-expansion of either vector.  So, for the '&' version to be
correct, it basically requires that length(omittedSig) = length(LHS) =
length(RHS) = sum(omittedSig), which also sounds unlikely to be the
original intention.

Disclaimer: Please note that I have not at all studied the rest of the
function, so the above is just based on that single line plus
debugging that 'omittedSig' is a logical vector.

Martin, I don't have the time to dive into this further.  Though I did
try to see if it happened when one of oligo's dependencies were
loaded, but that was not the case. It kicks in when oligo is loaded.
FYI, I can also replicate your non-replicatation on another R 3.6.0
version. I'll see if I can narrow down what's different, e.g.
comparing sessionInfo():s, etc.  However, I want to reply with
findings above asap due to the R 3.6.1 release and you might roll back
the patch (since it might introduce other bugs as Peter mentioned).

/Henrik


On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
<[hidden email]> wrote:

>
> >>>>> Henrik Bengtsson via R-core
> >>>>>     on Sun, 23 Jun 2019 11:29:58 -0700 writes:
>
>     > Thank you.
>     > To correct myself, I can indeed reproduce this with R --vanilla too.
>     > A reproducible example is:
>
>     > $ R --vanilla
>     > R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
>     > ...
>     >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     >> loadNamespace("oligo")
>     > Error in omittedSig && (signature[omittedSig] != "missing") :
>     > 'length(x) = 4 > 1' in coercion to 'logical(1)'
>     > Error: unable to load R code in package ‘oligo’
>
>     > /Henrik
>
> Thank you Henrik, for the report, etc, but
> hmm... after loading the oligo package, almost 40 (non
> base+Recommended) packages have been loaded as well, which hence
> need to have been installed before, too ..
> which is not quite a "vanilla repr.ex." in my view
>
> Worse, I cannot reproduce :
>
>     > Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     > Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
>     [1] "true"
>     > debugonce(conformMethod)
>     > loadNamespace("oligo")
>     <environment: namespace:oligo>
>     Warning messages:
>     1: multiple methods tables found for ‘rowSums’
>     2: multiple methods tables found for ‘colSums’
>     3: multiple methods tables found for ‘rowMeans’
>     4: multiple methods tables found for ‘colMeans’
>     > sessionInfo()
>     R Under development (unstable) (2019-06-20 r76729)
>
> (similarly with other versions of R >= 3.6.0).
>
> So, even though R core has fixed this now in the sources, it
> would be nice to have an "as simple as possible"  repr.ex. for this.
>
> Martin
>
>
>
>     > On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
>     >>
>     >> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>     >>
>     >> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>     >>
>     >> -pd
>     >>
>     >> > On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
>     >> >
>     >> > DISCLAIMER: I can not get this error with R --vanilla, so it only
>     >> > occurs when some other package is also loaded.  I don't have time to
>     >> > find to narrow that down for a reproducible example, but I believe the
>     >> > following error in R 3.6.0:
>     >> >
>     >> >> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     >> >> library(oligo)
>     >> > Error in omittedSig && (signature[omittedSig] != "missing") :
>     >> >  'length(x) = 4 > 1' in coercion to 'logical(1)'
>     >> > Error: unable to load R code in package 'oligo'
>     >> >
>     >> > is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
>     >> > 'methods' package.  Here's the patch:
>     >> >
>     >> > $ svn diff src/library/methods/R/RMethodUtils.R &
>     >> > [1] 1062
>     >> > Index: src/library/methods/R/RMethodUtils.R
>     >> > ===================================================================
>     >> > --- src/library/methods/R/RMethodUtils.R (revision 76731)
>     >> > +++ src/library/methods/R/RMethodUtils.R (working copy)
>     >> > @@ -343,7 +343,7 @@
>     >> >              call. = TRUE, domain = NA)
>     >> >     }
>     >> >     else if(!all(signature[omittedSig] == "missing")) {
>     >> > -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>     >> > +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>     >> >         .message("Note: ", .renderSignature(f, sig0),
>     >> >                  gettextf("expanding the signature to include omitted
>     >> > arguments in definition: %s",
>     >> >                           paste(sigNames[omittedSig], "=
>     >> > \"missing\"",collapse = ", ")))
>     >> > [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>     >> >
>     >> > Maybe still in time for R 3.6.1?
>     >> >
>     >> > /Henrik
>     >> >
>     >> > ______________________________________________
>     >> > [hidden email] mailing list
>     >> > https://stat.ethz.ch/mailman/listinfo/r-devel
>     >>
>     >> --
>     >> Peter Dalgaard, Professor,
>     >> Center for Statistics, Copenhagen Business School
>     >> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>     >> Phone: (+45)38153501
>     >> Office: A 4.23
>     >> Email: [hidden email]  Priv: [hidden email]
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Peter Dalgaard-2
Argh! Yes you are right, the "fix" doesn't. And I fell into the same "hey it's a vector so && has to be wrong"-trap. So this has to be reverted to something that has at least failed unconspicuously for a decade.... Will do. Thanks to Martin for remaining suspicious!

[This code was originally from 2009, by John Chambers. It is not too likely that he'll remember what the intention actually was... The logic does escape me: An omitted signature is only an omitted signature if the signature of the omitted signature is not "missing"? In that case, I think

omittedSig[omittedSig] <- (signature[omittedSig] != "missing")

might work (omittedSig[omittedSig] == TRUE, so we don't need to &. That is, unless there are NA issues.), but I am not at all sure that is what is wanted!
]

-pd

> On 25 Jun 2019, at 07:16 , Henrik Bengtsson <[hidden email]> wrote:
>
> **Maybe this bug needs to be understood further before applying the
> patch because patch is most likely also wrong**
>
> Because, from just looking at the expressions, I think neither the R
> 3.6.0 version:
>
> omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>
> nor the patched version (I proposed):
>
> omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>
> can be correct.  For a starter, 'omittedSig' is a logical vector.  We
> see that 'omittedSig' is used to subset 'signature'.  In other words,
> the length of 'signature[omittedSig]' and hence the length of
> '(signature[omittedSig] != "missing")' will equal sum(omittedSig),
> i.e. the length will be in {0,1,...,length(omittedSig)}.
>
> The R 3.6.0 version with '&&' is not correct because '&&' requires
> length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
> to be the original intention.
>
> The patched version with '&' is most likely not correct either because
> 'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
> auto-expansion of either vector.  So, for the '&' version to be
> correct, it basically requires that length(omittedSig) = length(LHS) =
> length(RHS) = sum(omittedSig), which also sounds unlikely to be the
> original intention.
>
> Disclaimer: Please note that I have not at all studied the rest of the
> function, so the above is just based on that single line plus
> debugging that 'omittedSig' is a logical vector.
>
> Martin, I don't have the time to dive into this further.  Though I did
> try to see if it happened when one of oligo's dependencies were
> loaded, but that was not the case. It kicks in when oligo is loaded.
> FYI, I can also replicate your non-replicatation on another R 3.6.0
> version. I'll see if I can narrow down what's different, e.g.
> comparing sessionInfo():s, etc.  However, I want to reply with
> findings above asap due to the R 3.6.1 release and you might roll back
> the patch (since it might introduce other bugs as Peter mentioned).
>
> /Henrik
>
>
> On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
> <[hidden email]> wrote:
>>
>>>>>>> Henrik Bengtsson via R-core
>>>>>>>    on Sun, 23 Jun 2019 11:29:58 -0700 writes:
>>
>>> Thank you.
>>> To correct myself, I can indeed reproduce this with R --vanilla too.
>>> A reproducible example is:
>>
>>> $ R --vanilla
>>> R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
>>> ...
>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>> loadNamespace("oligo")
>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>>> Error: unable to load R code in package ‘oligo’
>>
>>> /Henrik
>>
>> Thank you Henrik, for the report, etc, but
>> hmm... after loading the oligo package, almost 40 (non
>> base+Recommended) packages have been loaded as well, which hence
>> need to have been installed before, too ..
>> which is not quite a "vanilla repr.ex." in my view
>>
>> Worse, I cannot reproduce :
>>
>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>> Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
>>    [1] "true"
>>> debugonce(conformMethod)
>>> loadNamespace("oligo")
>>    <environment: namespace:oligo>
>>    Warning messages:
>>    1: multiple methods tables found for ‘rowSums’
>>    2: multiple methods tables found for ‘colSums’
>>    3: multiple methods tables found for ‘rowMeans’
>>    4: multiple methods tables found for ‘colMeans’
>>> sessionInfo()
>>    R Under development (unstable) (2019-06-20 r76729)
>>
>> (similarly with other versions of R >= 3.6.0).
>>
>> So, even though R core has fixed this now in the sources, it
>> would be nice to have an "as simple as possible"  repr.ex. for this.
>>
>> Martin
>>
>>
>>
>>> On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
>>>>
>>>> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>>>>
>>>> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>>>>
>>>> -pd
>>>>
>>>>> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
>>>>>
>>>>> DISCLAIMER: I can not get this error with R --vanilla, so it only
>>>>> occurs when some other package is also loaded.  I don't have time to
>>>>> find to narrow that down for a reproducible example, but I believe the
>>>>> following error in R 3.6.0:
>>>>>
>>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>>>> library(oligo)
>>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>>>>> Error: unable to load R code in package 'oligo'
>>>>>
>>>>> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
>>>>> 'methods' package.  Here's the patch:
>>>>>
>>>>> $ svn diff src/library/methods/R/RMethodUtils.R &
>>>>> [1] 1062
>>>>> Index: src/library/methods/R/RMethodUtils.R
>>>>> ===================================================================
>>>>> --- src/library/methods/R/RMethodUtils.R (revision 76731)
>>>>> +++ src/library/methods/R/RMethodUtils.R (working copy)
>>>>> @@ -343,7 +343,7 @@
>>>>>             call. = TRUE, domain = NA)
>>>>>    }
>>>>>    else if(!all(signature[omittedSig] == "missing")) {
>>>>> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>>>>> +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>>>>>        .message("Note: ", .renderSignature(f, sig0),
>>>>>                 gettextf("expanding the signature to include omitted
>>>>> arguments in definition: %s",
>>>>>                          paste(sigNames[omittedSig], "=
>>>>> \"missing\"",collapse = ", ")))
>>>>> [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>>>>>
>>>>> Maybe still in time for R 3.6.1?
>>>>>
>>>>> /Henrik
>>>>>
>>>>> ______________________________________________
>>>>> [hidden email] mailing list
>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>
>>>> --
>>>> Peter Dalgaard, Professor,
>>>> Center for Statistics, Copenhagen Business School
>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>>>> Phone: (+45)38153501
>>>> Office: A 4.23
>>>> Email: [hidden email]  Priv: [hidden email]
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Peter Dalgaard-2
Ah, not quite: The logic is that if something has omittedSig and "missing" is not the signature, those signatures get _set_ to missing. There's just a bit of tap-dancing around to find exactly which they are so that there can be a message about changing them.

-pd

> On 25 Jun 2019, at 09:44 , peter dalgaard <[hidden email]> wrote:
>
> Argh! Yes you are right, the "fix" doesn't. And I fell into the same "hey it's a vector so && has to be wrong"-trap. So this has to be reverted to something that has at least failed unconspicuously for a decade.... Will do. Thanks to Martin for remaining suspicious!
>
> [This code was originally from 2009, by John Chambers. It is not too likely that he'll remember what the intention actually was... The logic does escape me: An omitted signature is only an omitted signature if the signature of the omitted signature is not "missing"? In that case, I think
>
> omittedSig[omittedSig] <- (signature[omittedSig] != "missing")
>
> might work (omittedSig[omittedSig] == TRUE, so we don't need to &. That is, unless there are NA issues.), but I am not at all sure that is what is wanted!
> ]
>
> -pd
>
>> On 25 Jun 2019, at 07:16 , Henrik Bengtsson <[hidden email]> wrote:
>>
>> **Maybe this bug needs to be understood further before applying the
>> patch because patch is most likely also wrong**
>>
>> Because, from just looking at the expressions, I think neither the R
>> 3.6.0 version:
>>
>> omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>>
>> nor the patched version (I proposed):
>>
>> omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>>
>> can be correct.  For a starter, 'omittedSig' is a logical vector.  We
>> see that 'omittedSig' is used to subset 'signature'.  In other words,
>> the length of 'signature[omittedSig]' and hence the length of
>> '(signature[omittedSig] != "missing")' will equal sum(omittedSig),
>> i.e. the length will be in {0,1,...,length(omittedSig)}.
>>
>> The R 3.6.0 version with '&&' is not correct because '&&' requires
>> length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
>> to be the original intention.
>>
>> The patched version with '&' is most likely not correct either because
>> 'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
>> auto-expansion of either vector.  So, for the '&' version to be
>> correct, it basically requires that length(omittedSig) = length(LHS) =
>> length(RHS) = sum(omittedSig), which also sounds unlikely to be the
>> original intention.
>>
>> Disclaimer: Please note that I have not at all studied the rest of the
>> function, so the above is just based on that single line plus
>> debugging that 'omittedSig' is a logical vector.
>>
>> Martin, I don't have the time to dive into this further.  Though I did
>> try to see if it happened when one of oligo's dependencies were
>> loaded, but that was not the case. It kicks in when oligo is loaded.
>> FYI, I can also replicate your non-replicatation on another R 3.6.0
>> version. I'll see if I can narrow down what's different, e.g.
>> comparing sessionInfo():s, etc.  However, I want to reply with
>> findings above asap due to the R 3.6.1 release and you might roll back
>> the patch (since it might introduce other bugs as Peter mentioned).
>>
>> /Henrik
>>
>>
>> On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
>> <[hidden email]> wrote:
>>>
>>>>>>>> Henrik Bengtsson via R-core
>>>>>>>>   on Sun, 23 Jun 2019 11:29:58 -0700 writes:
>>>
>>>> Thank you.
>>>> To correct myself, I can indeed reproduce this with R --vanilla too.
>>>> A reproducible example is:
>>>
>>>> $ R --vanilla
>>>> R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
>>>> ...
>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>>> loadNamespace("oligo")
>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>>>> Error: unable to load R code in package ‘oligo’
>>>
>>>> /Henrik
>>>
>>> Thank you Henrik, for the report, etc, but
>>> hmm... after loading the oligo package, almost 40 (non
>>> base+Recommended) packages have been loaded as well, which hence
>>> need to have been installed before, too ..
>>> which is not quite a "vanilla repr.ex." in my view
>>>
>>> Worse, I cannot reproduce :
>>>
>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>> Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
>>>   [1] "true"
>>>> debugonce(conformMethod)
>>>> loadNamespace("oligo")
>>>   <environment: namespace:oligo>
>>>   Warning messages:
>>>   1: multiple methods tables found for ‘rowSums’
>>>   2: multiple methods tables found for ‘colSums’
>>>   3: multiple methods tables found for ‘rowMeans’
>>>   4: multiple methods tables found for ‘colMeans’
>>>> sessionInfo()
>>>   R Under development (unstable) (2019-06-20 r76729)
>>>
>>> (similarly with other versions of R >= 3.6.0).
>>>
>>> So, even though R core has fixed this now in the sources, it
>>> would be nice to have an "as simple as possible"  repr.ex. for this.
>>>
>>> Martin
>>>
>>>
>>>
>>>> On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
>>>>>
>>>>> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>>>>>
>>>>> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>>>>>
>>>>> -pd
>>>>>
>>>>>> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
>>>>>>
>>>>>> DISCLAIMER: I can not get this error with R --vanilla, so it only
>>>>>> occurs when some other package is also loaded.  I don't have time to
>>>>>> find to narrow that down for a reproducible example, but I believe the
>>>>>> following error in R 3.6.0:
>>>>>>
>>>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>>>>> library(oligo)
>>>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>>>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>>>>>> Error: unable to load R code in package 'oligo'
>>>>>>
>>>>>> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
>>>>>> 'methods' package.  Here's the patch:
>>>>>>
>>>>>> $ svn diff src/library/methods/R/RMethodUtils.R &
>>>>>> [1] 1062
>>>>>> Index: src/library/methods/R/RMethodUtils.R
>>>>>> ===================================================================
>>>>>> --- src/library/methods/R/RMethodUtils.R (revision 76731)
>>>>>> +++ src/library/methods/R/RMethodUtils.R (working copy)
>>>>>> @@ -343,7 +343,7 @@
>>>>>>            call. = TRUE, domain = NA)
>>>>>>   }
>>>>>>   else if(!all(signature[omittedSig] == "missing")) {
>>>>>> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>>>>>> +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>>>>>>       .message("Note: ", .renderSignature(f, sig0),
>>>>>>                gettextf("expanding the signature to include omitted
>>>>>> arguments in definition: %s",
>>>>>>                         paste(sigNames[omittedSig], "=
>>>>>> \"missing\"",collapse = ", ")))
>>>>>> [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>>>>>>
>>>>>> Maybe still in time for R 3.6.1?
>>>>>>
>>>>>> /Henrik
>>>>>>
>>>>>> ______________________________________________
>>>>>> [hidden email] mailing list
>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>
>>>>> --
>>>>> Peter Dalgaard, Professor,
>>>>> Center for Statistics, Copenhagen Business School
>>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>>>>> Phone: (+45)38153501
>>>>> Office: A 4.23
>>>>> Email: [hidden email]  Priv: [hidden email]
>>>
>>> ______________________________________________
>>> [hidden email] mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: [hidden email]  Priv: [hidden email]
>
>
>
>
>
>
>
>
>

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Peter Dalgaard-2
In reply to this post by Peter Dalgaard-2
Henrik,

If a minimal reprex is hard to construct, could you perhaps instrument your version of R to include a browser() call at the start of the    

else if(!all(signature[omittedSig] == "missing")) {

branch, run the code that triggers the issue for you (and must hit that branch) and tell us what the "signature" and  "omittedSig" objects look like at that point?

Bonus points for figuring out why it is needed to use numerical indexing in

        omittedSig <- seq_along(omittedSig)[omittedSig]
        signature[omittedSig] <- "missing" # logical index will extend signature!

(I'm sure there is a valid reason, I just don't get it...)

-pd

> On 25 Jun 2019, at 09:44 , peter dalgaard <[hidden email]> wrote:
>
> Argh! Yes you are right, the "fix" doesn't. And I fell into the same "hey it's a vector so && has to be wrong"-trap. So this has to be reverted to something that has at least failed unconspicuously for a decade.... Will do. Thanks to Martin for remaining suspicious!
>
> [This code was originally from 2009, by John Chambers. It is not too likely that he'll remember what the intention actually was... The logic does escape me: An omitted signature is only an omitted signature if the signature of the omitted signature is not "missing"? In that case, I think
>
> omittedSig[omittedSig] <- (signature[omittedSig] != "missing")
>
> might work (omittedSig[omittedSig] == TRUE, so we don't need to &. That is, unless there are NA issues.), but I am not at all sure that is what is wanted!
> ]
>
> -pd
>
>> On 25 Jun 2019, at 07:16 , Henrik Bengtsson <[hidden email]> wrote:
>>
>> **Maybe this bug needs to be understood further before applying the
>> patch because patch is most likely also wrong**
>>
>> Because, from just looking at the expressions, I think neither the R
>> 3.6.0 version:
>>
>> omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>>
>> nor the patched version (I proposed):
>>
>> omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>>
>> can be correct.  For a starter, 'omittedSig' is a logical vector.  We
>> see that 'omittedSig' is used to subset 'signature'.  In other words,
>> the length of 'signature[omittedSig]' and hence the length of
>> '(signature[omittedSig] != "missing")' will equal sum(omittedSig),
>> i.e. the length will be in {0,1,...,length(omittedSig)}.
>>
>> The R 3.6.0 version with '&&' is not correct because '&&' requires
>> length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
>> to be the original intention.
>>
>> The patched version with '&' is most likely not correct either because
>> 'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
>> auto-expansion of either vector.  So, for the '&' version to be
>> correct, it basically requires that length(omittedSig) = length(LHS) =
>> length(RHS) = sum(omittedSig), which also sounds unlikely to be the
>> original intention.
>>
>> Disclaimer: Please note that I have not at all studied the rest of the
>> function, so the above is just based on that single line plus
>> debugging that 'omittedSig' is a logical vector.
>>
>> Martin, I don't have the time to dive into this further.  Though I did
>> try to see if it happened when one of oligo's dependencies were
>> loaded, but that was not the case. It kicks in when oligo is loaded.
>> FYI, I can also replicate your non-replicatation on another R 3.6.0
>> version. I'll see if I can narrow down what's different, e.g.
>> comparing sessionInfo():s, etc.  However, I want to reply with
>> findings above asap due to the R 3.6.1 release and you might roll back
>> the patch (since it might introduce other bugs as Peter mentioned).
>>
>> /Henrik
>>
>>
>> On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
>> <[hidden email]> wrote:
>>>
>>>>>>>> Henrik Bengtsson via R-core
>>>>>>>>   on Sun, 23 Jun 2019 11:29:58 -0700 writes:
>>>
>>>> Thank you.
>>>> To correct myself, I can indeed reproduce this with R --vanilla too.
>>>> A reproducible example is:
>>>
>>>> $ R --vanilla
>>>> R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
>>>> ...
>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>>> loadNamespace("oligo")
>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>>>> Error: unable to load R code in package ‘oligo’
>>>
>>>> /Henrik
>>>
>>> Thank you Henrik, for the report, etc, but
>>> hmm... after loading the oligo package, almost 40 (non
>>> base+Recommended) packages have been loaded as well, which hence
>>> need to have been installed before, too ..
>>> which is not quite a "vanilla repr.ex." in my view
>>>
>>> Worse, I cannot reproduce :
>>>
>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>> Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
>>>   [1] "true"
>>>> debugonce(conformMethod)
>>>> loadNamespace("oligo")
>>>   <environment: namespace:oligo>
>>>   Warning messages:
>>>   1: multiple methods tables found for ‘rowSums’
>>>   2: multiple methods tables found for ‘colSums’
>>>   3: multiple methods tables found for ‘rowMeans’
>>>   4: multiple methods tables found for ‘colMeans’
>>>> sessionInfo()
>>>   R Under development (unstable) (2019-06-20 r76729)
>>>
>>> (similarly with other versions of R >= 3.6.0).
>>>
>>> So, even though R core has fixed this now in the sources, it
>>> would be nice to have an "as simple as possible"  repr.ex. for this.
>>>
>>> Martin
>>>
>>>
>>>
>>>> On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
>>>>>
>>>>> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>>>>>
>>>>> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>>>>>
>>>>> -pd
>>>>>
>>>>>> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
>>>>>>
>>>>>> DISCLAIMER: I can not get this error with R --vanilla, so it only
>>>>>> occurs when some other package is also loaded.  I don't have time to
>>>>>> find to narrow that down for a reproducible example, but I believe the
>>>>>> following error in R 3.6.0:
>>>>>>
>>>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>>>>> library(oligo)
>>>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>>>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>>>>>> Error: unable to load R code in package 'oligo'
>>>>>>
>>>>>> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
>>>>>> 'methods' package.  Here's the patch:
>>>>>>
>>>>>> $ svn diff src/library/methods/R/RMethodUtils.R &
>>>>>> [1] 1062
>>>>>> Index: src/library/methods/R/RMethodUtils.R
>>>>>> ===================================================================
>>>>>> --- src/library/methods/R/RMethodUtils.R (revision 76731)
>>>>>> +++ src/library/methods/R/RMethodUtils.R (working copy)
>>>>>> @@ -343,7 +343,7 @@
>>>>>>            call. = TRUE, domain = NA)
>>>>>>   }
>>>>>>   else if(!all(signature[omittedSig] == "missing")) {
>>>>>> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>>>>>> +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>>>>>>       .message("Note: ", .renderSignature(f, sig0),
>>>>>>                gettextf("expanding the signature to include omitted
>>>>>> arguments in definition: %s",
>>>>>>                         paste(sigNames[omittedSig], "=
>>>>>> \"missing\"",collapse = ", ")))
>>>>>> [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>>>>>>
>>>>>> Maybe still in time for R 3.6.1?
>>>>>>
>>>>>> /Henrik
>>>>>>
>>>>>> ______________________________________________
>>>>>> [hidden email] mailing list
>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>
>>>>> --
>>>>> Peter Dalgaard, Professor,
>>>>> Center for Statistics, Copenhagen Business School
>>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>>>>> Phone: (+45)38153501
>>>>> Office: A 4.23
>>>>> Email: [hidden email]  Priv: [hidden email]
>>>
>>> ______________________________________________
>>> [hidden email] mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: [hidden email]  Priv: [hidden email]
>
>
>
>
>
>
>
>
>

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> peter dalgaard
>>>>>     on Thu, 27 Jun 2019 16:23:14 +0200 writes:

    > Henrik,
    > If a minimal reprex is hard to construct, could you perhaps instrument your version of R to include a browser() call at the start of the    

    > else if(!all(signature[omittedSig] == "missing")) {

    > branch, run the code that triggers the issue for you (and must hit that branch) and tell us what the "signature" and  "omittedSig" objects look like at that point?

    > Bonus points for figuring out why it is needed to use numerical indexing in

    > omittedSig <- seq_along(omittedSig)[omittedSig]
    > signature[omittedSig] <- "missing" # logical index will extend signature!

    > (I'm sure there is a valid reason, I just don't get it...)

    > -pd

I've also have mused over that question...
and I had assumed some difference in the case the original
omittedSig contains NAs ... but that's NOT true actually, see:

  > sign2 <- signatures <- LETTERS
  > omittedSig <- LETTERS < "K"
  > omittedSig[c(8,18)] <- NA # now have an omittedSig with {T, F, NA}
  > iSig <- seq_along(omittedSig)[omittedSig]
  > sign2[iSig] <- "missing"
  > signatures[omittedSig] <- "missing"
  > identical(sign2, signatures)
  [1] TRUE
  >

so I still don't see the case where it makes a difference.

Martin

    >> On 25 Jun 2019, at 09:44 , peter dalgaard <[hidden email]> wrote:
    >>
    >> Argh! Yes you are right, the "fix" doesn't. And I fell into the same "hey it's a vector so && has to be wrong"-trap. So this has to be reverted to something that has at least failed unconspicuously for a decade.... Will do. Thanks to Martin for remaining suspicious!
    >>
    >> [This code was originally from 2009, by John Chambers. It is not too likely that he'll remember what the intention actually was... The logic does escape me: An omitted signature is only an omitted signature if the signature of the omitted signature is not "missing"? In that case, I think
    >>
    >> omittedSig[omittedSig] <- (signature[omittedSig] != "missing")
    >>
    >> might work (omittedSig[omittedSig] == TRUE, so we don't need to &. That is, unless there are NA issues.), but I am not at all sure that is what is wanted!
    >> ]
    >>
    >> -pd
    >>
    >>> On 25 Jun 2019, at 07:16 , Henrik Bengtsson <[hidden email]> wrote:
    >>>
    >>> **Maybe this bug needs to be understood further before applying the
    >>> patch because patch is most likely also wrong**
    >>>
    >>> Because, from just looking at the expressions, I think neither the R
    >>> 3.6.0 version:
    >>>
    >>> omittedSig <- omittedSig && (signature[omittedSig] != "missing")
    >>>
    >>> nor the patched version (I proposed):
    >>>
    >>> omittedSig <- omittedSig & (signature[omittedSig] != "missing")
    >>>
    >>> can be correct.  For a starter, 'omittedSig' is a logical vector.  We
    >>> see that 'omittedSig' is used to subset 'signature'.  In other words,
    >>> the length of 'signature[omittedSig]' and hence the length of
    >>> '(signature[omittedSig] != "missing")' will equal sum(omittedSig),
    >>> i.e. the length will be in {0,1,...,length(omittedSig)}.
    >>>
    >>> The R 3.6.0 version with '&&' is not correct because '&&' requires
    >>> length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
    >>> to be the original intention.
    >>>
    >>> The patched version with '&' is most likely not correct either because
    >>> 'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
    >>> auto-expansion of either vector.  So, for the '&' version to be
    >>> correct, it basically requires that length(omittedSig) = length(LHS) =
    >>> length(RHS) = sum(omittedSig), which also sounds unlikely to be the
    >>> original intention.
    >>>
    >>> Disclaimer: Please note that I have not at all studied the rest of the
    >>> function, so the above is just based on that single line plus
    >>> debugging that 'omittedSig' is a logical vector.
    >>>
    >>> Martin, I don't have the time to dive into this further.  Though I did
    >>> try to see if it happened when one of oligo's dependencies were
    >>> loaded, but that was not the case. It kicks in when oligo is loaded.
    >>> FYI, I can also replicate your non-replicatation on another R 3.6.0
    >>> version. I'll see if I can narrow down what's different, e.g.
    >>> comparing sessionInfo():s, etc.  However, I want to reply with
    >>> findings above asap due to the R 3.6.1 release and you might roll back
    >>> the patch (since it might introduce other bugs as Peter mentioned).
    >>>
    >>> /Henrik
    >>>
    >>>
    >>> On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
    >>> <[hidden email]> wrote:
    >>>>
    >>>>>>>>> Henrik Bengtsson via R-core
    >>>>>>>>> on Sun, 23 Jun 2019 11:29:58 -0700 writes:
    >>>>
    >>>>> Thank you.
    >>>>> To correct myself, I can indeed reproduce this with R --vanilla too.
    >>>>> A reproducible example is:
    >>>>
    >>>>> $ R --vanilla
    >>>>> R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
    >>>>> ...
>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>>>>> loadNamespace("oligo")
    >>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
    >>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
    >>>>> Error: unable to load R code in package ‘oligo’
    >>>>
    >>>>> /Henrik
    >>>>
    >>>> Thank you Henrik, for the report, etc, but
    >>>> hmm... after loading the oligo package, almost 40 (non
    >>>> base+Recommended) packages have been loaded as well, which hence
    >>>> need to have been installed before, too ..
    >>>> which is not quite a "vanilla repr.ex." in my view
    >>>>
    >>>> Worse, I cannot reproduce :
    >>>>
    >>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
    >>>>> Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
    >>>> [1] "true"
    >>>>> debugonce(conformMethod)
    >>>>> loadNamespace("oligo")
    >>>> <environment: namespace:oligo>
    >>>> Warning messages:
    >>>> 1: multiple methods tables found for ‘rowSums’
    >>>> 2: multiple methods tables found for ‘colSums’
    >>>> 3: multiple methods tables found for ‘rowMeans’
    >>>> 4: multiple methods tables found for ‘colMeans’
    >>>>> sessionInfo()
    >>>> R Under development (unstable) (2019-06-20 r76729)
    >>>>
    >>>> (similarly with other versions of R >= 3.6.0).
    >>>>
    >>>> So, even though R core has fixed this now in the sources, it
    >>>> would be nice to have an "as simple as possible"  repr.ex. for this.
    >>>>
    >>>> Martin
    >>>>
    >>>>
    >>>>
    >>>>> On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
>>>>>
>>>>> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
>>>>>
>>>>> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
>>>>>
>>>>> -pd
>>>>>
    >>>>>>> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
    >>>>>>>
    >>>>>>> DISCLAIMER: I can not get this error with R --vanilla, so it only
    >>>>>>> occurs when some other package is also loaded.  I don't have time to
    >>>>>>> find to narrow that down for a reproducible example, but I believe the
    >>>>>>> following error in R 3.6.0:
    >>>>>>>
    >>>>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
    >>>>>>>> library(oligo)
    >>>>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
    >>>>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
    >>>>>>> Error: unable to load R code in package 'oligo'
    >>>>>>>
    >>>>>>> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
    >>>>>>> 'methods' package.  Here's the patch:
    >>>>>>>
    >>>>>>> $ svn diff src/library/methods/R/RMethodUtils.R &
    >>>>>>> [1] 1062
    >>>>>>> Index: src/library/methods/R/RMethodUtils.R
    >>>>>>> ===================================================================
    >>>>>>> --- src/library/methods/R/RMethodUtils.R (revision 76731)
    >>>>>>> +++ src/library/methods/R/RMethodUtils.R (working copy)
    >>>>>>> @@ -343,7 +343,7 @@
    >>>>>>> call. = TRUE, domain = NA)
    >>>>>>> }
    >>>>>>> else if(!all(signature[omittedSig] == "missing")) {
    >>>>>>> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
    >>>>>>> +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
    >>>>>>> .message("Note: ", .renderSignature(f, sig0),
    >>>>>>> gettextf("expanding the signature to include omitted
    >>>>>>> arguments in definition: %s",
    >>>>>>> paste(sigNames[omittedSig], "=
    >>>>>>> \"missing\"",collapse = ", ")))
    >>>>>>> [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
    >>>>>>>
    >>>>>>> Maybe still in time for R 3.6.1?
    >>>>>>>
    >>>>>>> /Henrik
    >>>>>>>
    >>>>>>> ______________________________________________
    >>>>>>> [hidden email] mailing list
    >>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>
>>>>> --
>>>>> Peter Dalgaard, Professor,
>>>>> Center for Statistics, Copenhagen Business School
>>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>>>>> Phone: (+45)38153501
>>>>> Office: A 4.23
>>>>> Email: [hidden email]  Priv: [hidden email]
    >>>>
    >>>> ______________________________________________
    >>>> [hidden email] mailing list
    >>>> https://stat.ethz.ch/mailman/listinfo/r-devel
    >>>
    >>> ______________________________________________
    >>> [hidden email] mailing list
    >>> https://stat.ethz.ch/mailman/listinfo/r-devel
    >>
    >> --
    >> Peter Dalgaard, Professor,
    >> Center for Statistics, Copenhagen Business School
    >> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
    >> Phone: (+45)38153501
    >> Office: A 4.23
    >> Email: [hidden email]  Priv: [hidden email]
    >>
    >>
    >>
    >>
    >>
    >>
    >>
    >>
    >>

    > --
    > Peter Dalgaard, Professor,
    > Center for Statistics, Copenhagen Business School
    > Solbjerg Plads 3, 2000 Frederiksberg, Denmark
    > Phone: (+45)38153501
    > Office: A 4.23
    > Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Henrik Bengtsson-5
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'

This is with:

sessionInfo()
R version 3.6.0 Patched (2019-06-23 r76734)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 18.04.2 LTS

Matrix products: default
BLAS:   /home/hb/software/R-devel/R-3-6-branch/lib/R/lib/libRblas.so
LAPACK: /home/hb/software/R-devel/R-3-6-branch/lib/R/lib/libRlapack.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.1                  affxparser_1.56.0
 [3] XVector_0.24.0              splines_3.6.0
 [5] GenomicRanges_1.36.0        BiocGenerics_0.30.0
 [7] zlibbioc_1.30.0             IRanges_2.18.1
 [9] bit_1.1-14                  BiocParallel_1.18.0
[11] lattice_0.20-38             foreach_1.4.4
[13] blob_1.1.1                  GenomeInfoDb_1.20.0
[15] tools_3.6.0                 SummarizedExperiment_1.14.0
[17] parallel_3.6.0              grid_3.6.0
[19] Biobase_2.44.0              ff_2.2-14
[21] DBI_1.0.0                   iterators_1.0.10
[23] oligoClasses_1.46.0         matrixStats_0.54.0
[25] digest_0.6.19               bit64_0.9-7
[27] preprocessCore_1.46.0       affyio_1.54.0
[29] Matrix_1.2-17               GenomeInfoDbData_1.2.1
[31] BiocManager_1.30.4          codetools_0.2-16
[33] S4Vectors_0.22.0            bitops_1.0-6
[35] RCurl_1.95-4.12             memoise_1.1.0
[37] RSQLite_2.1.1               DelayedArray_0.10.0
[39] compiler_3.6.0              Biostrings_2.52.0
[41] stats4_3.6.0

/Henrik

On Thu, Jun 27, 2019 at 8:16 AM Martin Maechler
<[hidden email]> wrote:

>
> >>>>> peter dalgaard
> >>>>>     on Thu, 27 Jun 2019 16:23:14 +0200 writes:
>
>     > Henrik,
>     > If a minimal reprex is hard to construct, could you perhaps instrument your version of R to include a browser() call at the start of the
>
>     > else if(!all(signature[omittedSig] == "missing")) {
>
>     > branch, run the code that triggers the issue for you (and must hit that branch) and tell us what the "signature" and  "omittedSig" objects look like at that point?
>
>     > Bonus points for figuring out why it is needed to use numerical indexing in
>
>     > omittedSig <- seq_along(omittedSig)[omittedSig]
>     > signature[omittedSig] <- "missing" # logical index will extend signature!
>
>     > (I'm sure there is a valid reason, I just don't get it...)
>
>     > -pd
>
> I've also have mused over that question...
> and I had assumed some difference in the case the original
> omittedSig contains NAs ... but that's NOT true actually, see:
>
>   > sign2 <- signatures <- LETTERS
>   > omittedSig <- LETTERS < "K"
>   > omittedSig[c(8,18)] <- NA # now have an omittedSig with {T, F, NA}
>   > iSig <- seq_along(omittedSig)[omittedSig]
>   > sign2[iSig] <- "missing"
>   > signatures[omittedSig] <- "missing"
>   > identical(sign2, signatures)
>   [1] TRUE
>   >
>
> so I still don't see the case where it makes a difference.
>
> Martin
>
>     >> On 25 Jun 2019, at 09:44 , peter dalgaard <[hidden email]> wrote:
>     >>
>     >> Argh! Yes you are right, the "fix" doesn't. And I fell into the same "hey it's a vector so && has to be wrong"-trap. So this has to be reverted to something that has at least failed unconspicuously for a decade.... Will do. Thanks to Martin for remaining suspicious!
>     >>
>     >> [This code was originally from 2009, by John Chambers. It is not too likely that he'll remember what the intention actually was... The logic does escape me: An omitted signature is only an omitted signature if the signature of the omitted signature is not "missing"? In that case, I think
>     >>
>     >> omittedSig[omittedSig] <- (signature[omittedSig] != "missing")
>     >>
>     >> might work (omittedSig[omittedSig] == TRUE, so we don't need to &. That is, unless there are NA issues.), but I am not at all sure that is what is wanted!
>     >> ]
>     >>
>     >> -pd
>     >>
>     >>> On 25 Jun 2019, at 07:16 , Henrik Bengtsson <[hidden email]> wrote:
>     >>>
>     >>> **Maybe this bug needs to be understood further before applying the
>     >>> patch because patch is most likely also wrong**
>     >>>
>     >>> Because, from just looking at the expressions, I think neither the R
>     >>> 3.6.0 version:
>     >>>
>     >>> omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>     >>>
>     >>> nor the patched version (I proposed):
>     >>>
>     >>> omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>     >>>
>     >>> can be correct.  For a starter, 'omittedSig' is a logical vector.  We
>     >>> see that 'omittedSig' is used to subset 'signature'.  In other words,
>     >>> the length of 'signature[omittedSig]' and hence the length of
>     >>> '(signature[omittedSig] != "missing")' will equal sum(omittedSig),
>     >>> i.e. the length will be in {0,1,...,length(omittedSig)}.
>     >>>
>     >>> The R 3.6.0 version with '&&' is not correct because '&&' requires
>     >>> length(omittedSig) == 1L and sum(omittedSig) == 1L, which is unlikely
>     >>> to be the original intention.
>     >>>
>     >>> The patched version with '&' is most likely not correct either because
>     >>> 'LHS & RHS' assume length(LHS) == length(RHS), unless it relies on the
>     >>> auto-expansion of either vector.  So, for the '&' version to be
>     >>> correct, it basically requires that length(omittedSig) = length(LHS) =
>     >>> length(RHS) = sum(omittedSig), which also sounds unlikely to be the
>     >>> original intention.
>     >>>
>     >>> Disclaimer: Please note that I have not at all studied the rest of the
>     >>> function, so the above is just based on that single line plus
>     >>> debugging that 'omittedSig' is a logical vector.
>     >>>
>     >>> Martin, I don't have the time to dive into this further.  Though I did
>     >>> try to see if it happened when one of oligo's dependencies were
>     >>> loaded, but that was not the case. It kicks in when oligo is loaded.
>     >>> FYI, I can also replicate your non-replicatation on another R 3.6.0
>     >>> version. I'll see if I can narrow down what's different, e.g.
>     >>> comparing sessionInfo():s, etc.  However, I want to reply with
>     >>> findings above asap due to the R 3.6.1 release and you might roll back
>     >>> the patch (since it might introduce other bugs as Peter mentioned).
>     >>>
>     >>> /Henrik
>     >>>
>     >>>
>     >>> On Mon, Jun 24, 2019 at 3:05 AM Martin Maechler
>     >>> <[hidden email]> wrote:
>     >>>>
>     >>>>>>>>> Henrik Bengtsson via R-core
>     >>>>>>>>> on Sun, 23 Jun 2019 11:29:58 -0700 writes:
>     >>>>
>     >>>>> Thank you.
>     >>>>> To correct myself, I can indeed reproduce this with R --vanilla too.
>     >>>>> A reproducible example is:
>     >>>>
>     >>>>> $ R --vanilla
>     >>>>> R version 3.6.0 Patched (2019-05-31 r76629) -- "Planting of a Tree"
>     >>>>> ...
> >>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
> >>>>> loadNamespace("oligo")
>     >>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>     >>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>     >>>>> Error: unable to load R code in package ‘oligo’
>     >>>>
>     >>>>> /Henrik
>     >>>>
>     >>>> Thank you Henrik, for the report, etc, but
>     >>>> hmm... after loading the oligo package, almost 40 (non
>     >>>> base+Recommended) packages have been loaded as well, which hence
>     >>>> need to have been installed before, too ..
>     >>>> which is not quite a "vanilla repr.ex." in my view
>     >>>>
>     >>>> Worse, I cannot reproduce :
>     >>>>
>     >>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     >>>>> Sys.getenv("_R_CHECK_LENGTH_1_LOGIC2_")
>     >>>> [1] "true"
>     >>>>> debugonce(conformMethod)
>     >>>>> loadNamespace("oligo")
>     >>>> <environment: namespace:oligo>
>     >>>> Warning messages:
>     >>>> 1: multiple methods tables found for ‘rowSums’
>     >>>> 2: multiple methods tables found for ‘colSums’
>     >>>> 3: multiple methods tables found for ‘rowMeans’
>     >>>> 4: multiple methods tables found for ‘colMeans’
>     >>>>> sessionInfo()
>     >>>> R Under development (unstable) (2019-06-20 r76729)
>     >>>>
>     >>>> (similarly with other versions of R >= 3.6.0).
>     >>>>
>     >>>> So, even though R core has fixed this now in the sources, it
>     >>>> would be nice to have an "as simple as possible"  repr.ex. for this.
>     >>>>
>     >>>> Martin
>     >>>>
>     >>>>
>     >>>>
>     >>>>> On Sun, Jun 23, 2019 at 1:54 AM peter dalgaard <[hidden email]> wrote:
> >>>>>
> >>>>> This looks obvious enough, so I just committed your fix to R-devel and R-patched.
> >>>>>
> >>>>> I'm at the wrong machine for thorough testing, but at least it seems to build OK. However, I sense some risk that this could uncover sleeping bugs elsewhere, so watch out.
> >>>>>
> >>>>> -pd
> >>>>>
>     >>>>>>> On 22 Jun 2019, at 18:49 , Henrik Bengtsson <[hidden email]> wrote:
>     >>>>>>>
>     >>>>>>> DISCLAIMER: I can not get this error with R --vanilla, so it only
>     >>>>>>> occurs when some other package is also loaded.  I don't have time to
>     >>>>>>> find to narrow that down for a reproducible example, but I believe the
>     >>>>>>> following error in R 3.6.0:
>     >>>>>>>
>     >>>>>>>> Sys.setenv("_R_CHECK_LENGTH_1_LOGIC2_" = "true")
>     >>>>>>>> library(oligo)
>     >>>>>>> Error in omittedSig && (signature[omittedSig] != "missing") :
>     >>>>>>> 'length(x) = 4 > 1' in coercion to 'logical(1)'
>     >>>>>>> Error: unable to load R code in package 'oligo'
>     >>>>>>>
>     >>>>>>> is because of a '_R_CHECK_LENGTH_1_LOGIC2_=true' mistake in the
>     >>>>>>> 'methods' package.  Here's the patch:
>     >>>>>>>
>     >>>>>>> $ svn diff src/library/methods/R/RMethodUtils.R &
>     >>>>>>> [1] 1062
>     >>>>>>> Index: src/library/methods/R/RMethodUtils.R
>     >>>>>>> ===================================================================
>     >>>>>>> --- src/library/methods/R/RMethodUtils.R (revision 76731)
>     >>>>>>> +++ src/library/methods/R/RMethodUtils.R (working copy)
>     >>>>>>> @@ -343,7 +343,7 @@
>     >>>>>>> call. = TRUE, domain = NA)
>     >>>>>>> }
>     >>>>>>> else if(!all(signature[omittedSig] == "missing")) {
>     >>>>>>> -        omittedSig <- omittedSig && (signature[omittedSig] != "missing")
>     >>>>>>> +        omittedSig <- omittedSig & (signature[omittedSig] != "missing")
>     >>>>>>> .message("Note: ", .renderSignature(f, sig0),
>     >>>>>>> gettextf("expanding the signature to include omitted
>     >>>>>>> arguments in definition: %s",
>     >>>>>>> paste(sigNames[omittedSig], "=
>     >>>>>>> \"missing\"",collapse = ", ")))
>     >>>>>>> [1]+  Done                    svn diff src/library/methods/R/RMethodUtils.R
>     >>>>>>>
>     >>>>>>> Maybe still in time for R 3.6.1?
>     >>>>>>>
>     >>>>>>> /Henrik
>     >>>>>>>
>     >>>>>>> ______________________________________________
>     >>>>>>> [hidden email] mailing list
>     >>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >>>>>
> >>>>> --
> >>>>> Peter Dalgaard, Professor,
> >>>>> Center for Statistics, Copenhagen Business School
> >>>>> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> >>>>> Phone: (+45)38153501
> >>>>> Office: A 4.23
> >>>>> Email: [hidden email]  Priv: [hidden email]
>     >>>>
>     >>>> ______________________________________________
>     >>>> [hidden email] mailing list
>     >>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>     >>>
>     >>> ______________________________________________
>     >>> [hidden email] mailing list
>     >>> https://stat.ethz.ch/mailman/listinfo/r-devel
>     >>
>     >> --
>     >> Peter Dalgaard, Professor,
>     >> Center for Statistics, Copenhagen Business School
>     >> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>     >> Phone: (+45)38153501
>     >> Office: A 4.23
>     >> Email: [hidden email]  Priv: [hidden email]
>     >>
>     >>
>     >>
>     >>
>     >>
>     >>
>     >>
>     >>
>     >>
>
>     > --
>     > Peter Dalgaard, Professor,
>     > Center for Statistics, Copenhagen Business School
>     > Solbjerg Plads 3, 2000 Frederiksberg, Denmark
>     > Phone: (+45)38153501
>     > Office: A 4.23
>     > Email: [hidden email]  Priv: [hidden email]
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> 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] # ....

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


*Still*  I cannot understand why in my case (and probably Peter,
 as he also said he can't reproduce),
the  conformMethod() function is not even called  when I run
     loadNamespace("oligo").

As conformMethod() is *only* called from setMethod(), I've
started trace()ing  setMethod() and indeed, it is *only* called one,
and not with oligo methods/generics,...

Henrik, do you per chance not install packages in the usual way,
i.e., do you install them without saving all the pre-computed
classes and methods tables etc,
and that would be *why* these setMethod() etc are only called at
this point in time ?

Martin

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Peter Dalgaard-2


> On 28 Jun 2019, at 16:03 , Martin Maechler <[hidden email]> 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] # ....
>
> -------------
>

I think the intention must have been that the two "ANY" signatures should change to "missing". 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.

-pd

>
> *Still*  I cannot understand why in my case (and probably Peter,
> as he also said he can't reproduce),
> the  conformMethod() function is not even called  when I run
>     loadNamespace("oligo").
>
> As conformMethod() is *only* called from setMethod(), I've
> started trace()ing  setMethod() and indeed, it is *only* called one,
> and not with oligo methods/generics,...
>
> Henrik, do you per chance not install packages in the usual way,
> i.e., do you install them without saving all the pre-computed
> classes and methods tables etc,
> and that would be *why* these setMethod() etc are only called at
> this point in time ?
>
> Martin

--
Peter Dalgaard, Professor,
Center for Statistics, Copenhagen Business School
Solbjerg Plads 3, 2000 Frederiksberg, Denmark
Phone: (+45)38153501
Office: A 4.23
Email: [hidden email]  Priv: [hidden email]

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> peter dalgaard
>>>>>     on Fri, 28 Jun 2019 16:20:03 +0200 writes:

> > On 28 Jun 2019, at 16:03 , Martin Maechler <[hidden email]> 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


> > *Still*  I cannot understand why in my case (and probably Peter,
> > as he also said he can't reproduce),
> > the  conformMethod() function is not even called  when I run
> >     loadNamespace("oligo").
> >
> > As conformMethod() is *only* called from setMethod(), I've
> > started trace()ing  setMethod() and indeed, it is *only* called one,
> > and not with oligo methods/generics,...
> >
> > Henrik, do you per chance not install packages in the usual way,
> > i.e., do you install them without saving all the pre-computed
> > classes and methods tables etc,
> > and that would be *why* these setMethod() etc are only called at
> > this point in time ?
> >
> > Martin
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: [hidden email]  Priv: [hidden email]
>
>
>
>
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> 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 <[hidden email]> 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

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> 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 <[hidden email]> 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

    > ______________________________________________
    > [hidden email] mailing list
    > https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

R devel mailing list
In reply to this post by Henrik Bengtsson-5
In 'conformMethod', there is another instance of
omittedSig & <something about signature[omittedSig]> .
It just affects error message.

Original:
    if(any(is.na(match(signature[omittedSig], c("ANY", "missing"))))) {
        bad <- omittedSig & is.na(match(signature[omittedSig], c("ANY", "missing")))

After r76756:
    if(any(iiN <- is.na(match(signature[omittedSig], c("ANY", "missing"))))) {
        bad <- omittedSig & iiN


----------------------------------------------
>>>>> 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 using 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

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Martin Maechler
>>>>> Suharto Anggono Suharto Anggono via R-devel
>>>>>     on Thu, 4 Jul 2019 15:13:55 +0000 writes:

    > In 'conformMethod', there is another instance of
    > omittedSig & <something about signature[omittedSig]> .
    > It just affects error message.

    > Original:
    >     if(any(is.na(match(signature[omittedSig], c("ANY", "missing"))))) {
    >         bad <- omittedSig & is.na(match(signature[omittedSig], c("ANY", "missing")))

    > After r76756:
    >     if(any(iiN <- is.na(match(signature[omittedSig], c("ANY", "missing"))))) {
    >         bad <- omittedSig & iiN

Yes, thank you, that's same "internal logic" error as later.

One difference is that I think the above case cannot be
triggered by setMethod() .. one needs an explicit "artificial"
conformMethod() call.
Still, of course, it's wrong and should be corrected.
I've started to test changes .. maye not get to commit (with
enough confidence) before taking the train to
useR! 2019 @  Toulouse (France).

Martin

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: methods package: A _R_CHECK_LENGTH_1_LOGIC2_=true error

Henrik Bengtsson-5
In reply to this post by Martin Maechler
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
<[hidden email]> 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 <[hidden email]> 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
>
>     > ______________________________________________
>     > [hidden email] mailing list
>     > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel