R devel - Apr 2016 - S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

See also .isMethodsDispatchOn, which is what trace uses to decide if the
methods package needs to be loaded.

~G

On Tue, Apr 19, 2016 at 5:34 AM, Michael Lawrence <lawrence.michael at
gene.com
> wrote:
> Not sure why R_has_methods_attached() exists. Maybe Martin could shed
> some light on that.
>
> On Mon, Apr 18, 2016 at 11:50 PM, Kirill M?ller
> <kirill.mueller at ivt.baug.ethz.ch> wrote:
> > Thanks for looking into it, your approach sounds good to me. See also
> > R_has_methods_attached()
> > (
>
https://github.com/wch/r-source/blob/42ecf5f492a005f5398cbb4c9becd4aa5af9d05c/src/main/objects.c#L258-L265
> ).
> >
> > I'm fine with Rscript not loading "methods", as long as
everything works
> > properly with "methods" loaded but not attached.
> >
> >
> > -Kirill
> >
> >
> >
> > On 19.04.2016 04:10, Michael Lawrence wrote:
> >>
> >> Right, the methods package is not attached by default when running
R
> >> with Rscript. We should probably remove that special case, as it
> >> mostly just leads to confusion, but that won't happen
immediately.
> >>
> >> For now, the S4_extends() should probably throw an error when the
> >> methods namespace is not loaded. And the check should be changed
to
> >> directly check whether R_MethodsNamespace has been set to
something
> >> other than the default (R_GlobalEnv). Agreed?
> >>
> >> On Mon, Apr 18, 2016 at 4:35 PM, Kirill M?ller
> >> <kirill.mueller at ivt.baug.ethz.ch> wrote:
> >>>
> >>> Scenario: An S3 method is declared for an S4 base class but
called for
> an
> >>> instance of a derived class.
> >>>
> >>> Steps to reproduce:
> >>>
> >>>> Rscript -e "test <- function(x)
UseMethod('test', x); test.Matrix <-
> >>>> function(x) 'Hi';
MatrixDispatchTest::test(Matrix::Matrix())"
> >>>
> >>> Error in UseMethod("test", x) :
> >>>    no applicable method for 'test' applied to an
object of class
> >>> "lsyMatrix"
> >>> Calls: <Anonymous>
> >>> 1: MatrixDispatchTest::test(Matrix::Matrix())
> >>>
> >>>> Rscript -e "extends <- 42; test <- function(x)
UseMethod('test', x);
> >>>> test.Matrix <- function(x) 'Hi';
> >>>> MatrixDispatchTest::test(Matrix::Matrix())"
> >>>
> >>> [1] "Hi"
> >>>
> >>> To me, it looks like a sanity check in line 655 of
src/main/attrib.c is
> >>> making wrong assumptions, but there might be other reasons.
> >>>
> >>> (
>
https://github.com/wch/r-source/blob/780021752eb83a71e2198019acf069ba8741103b/src/main/attrib.c#L655-L656
> )
> >>>
> >>> Same behavior in R 3.2.4, R 3.2.5 and R-devel r70420.
> >>>
> >>>
> >>> Best regards
> >>>
> >>> Kirill
> >>>
> >>> ______________________________________________
> >>> R-devel at r-project.org mailing list
> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >>>
> >
> >
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>


-- 
Gabriel Becker, PhD
Associate Scientist (Bioinformatics)
Genentech Research

	[[alternative HTML version deleted]]

Right, R_has_methods_attached() uses that. Probably not the right
check, since it refers to S4 dispatch, while S4_extends() is used by
S3 dispatch.

Perhaps S4_extends() should force load the methods package? The above
example works after fixing the check to ensure that R_MethodsNamespace
is not R_GlobalEnv, but one could load a serialized S4 object and
expect S3 dispatch to work with Rscript.

On Tue, Apr 19, 2016 at 6:51 AM, Gabriel Becker <gmbecker at ucdavis.edu>
wrote:
> See also .isMethodsDispatchOn, which is what trace uses to decide if the
> methods package needs to be loaded.
>
> ~G
>
> On Tue, Apr 19, 2016 at 5:34 AM, Michael Lawrence
> <lawrence.michael at gene.com> wrote:
>>
>> Not sure why R_has_methods_attached() exists. Maybe Martin could shed
>> some light on that.
>>
>> On Mon, Apr 18, 2016 at 11:50 PM, Kirill M?ller
>> <kirill.mueller at ivt.baug.ethz.ch> wrote:
>> > Thanks for looking into it, your approach sounds good to me. See
also
>> > R_has_methods_attached()
>> >
>> >
(https://github.com/wch/r-source/blob/42ecf5f492a005f5398cbb4c9becd4aa5af9d05c/src/main/objects.c#L258-L265).
>> >
>> > I'm fine with Rscript not loading "methods", as long
as everything works
>> > properly with "methods" loaded but not attached.
>> >
>> >
>> > -Kirill
>> >
>> >
>> >
>> > On 19.04.2016 04:10, Michael Lawrence wrote:
>> >>
>> >> Right, the methods package is not attached by default when
running R
>> >> with Rscript. We should probably remove that special case, as
it
>> >> mostly just leads to confusion, but that won't happen
immediately.
>> >>
>> >> For now, the S4_extends() should probably throw an error when
the
>> >> methods namespace is not loaded. And the check should be
changed to
>> >> directly check whether R_MethodsNamespace has been set to
something
>> >> other than the default (R_GlobalEnv). Agreed?
>> >>
>> >> On Mon, Apr 18, 2016 at 4:35 PM, Kirill M?ller
>> >> <kirill.mueller at ivt.baug.ethz.ch> wrote:
>> >>>
>> >>> Scenario: An S3 method is declared for an S4 base class
but called for
>> >>> an
>> >>> instance of a derived class.
>> >>>
>> >>> Steps to reproduce:
>> >>>
>> >>>> Rscript -e "test <- function(x)
UseMethod('test', x); test.Matrix <-
>> >>>> function(x) 'Hi';
MatrixDispatchTest::test(Matrix::Matrix())"
>> >>>
>> >>> Error in UseMethod("test", x) :
>> >>>    no applicable method for 'test' applied to an
object of class
>> >>> "lsyMatrix"
>> >>> Calls: <Anonymous>
>> >>> 1: MatrixDispatchTest::test(Matrix::Matrix())
>> >>>
>> >>>> Rscript -e "extends <- 42; test <-
function(x) UseMethod('test', x);
>> >>>> test.Matrix <- function(x) 'Hi';
>> >>>> MatrixDispatchTest::test(Matrix::Matrix())"
>> >>>
>> >>> [1] "Hi"
>> >>>
>> >>> To me, it looks like a sanity check in line 655 of
src/main/attrib.c
>> >>> is
>> >>> making wrong assumptions, but there might be other
reasons.
>> >>>
>> >>>
>> >>>
(https://github.com/wch/r-source/blob/780021752eb83a71e2198019acf069ba8741103b/src/main/attrib.c#L655-L656)
>> >>>
>> >>> Same behavior in R 3.2.4, R 3.2.5 and R-devel r70420.
>> >>>
>> >>>
>> >>> Best regards
>> >>>
>> >>> Kirill
>> >>>
>> >>> ______________________________________________
>> >>> R-devel at r-project.org mailing list
>> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> >>>
>> >
>> >
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>
>
>
> --
> Gabriel Becker, PhD
> Associate Scientist (Bioinformatics)
> Genentech Research

Does it make sense to be able to load an S4 object without the methods
package being attached? I'm not sure implementation-wise how easy this
would be, but it seems like any time there is an S4 object around, the
methods package should be available to deal with it.

~G

On Tue, Apr 19, 2016 at 7:34 AM, Michael Lawrence <lawrence.michael at
gene.com
> wrote:
> Right, R_has_methods_attached() uses that. Probably not the right
> check, since it refers to S4 dispatch, while S4_extends() is used by
> S3 dispatch.
>
> Perhaps S4_extends() should force load the methods package? The above
> example works after fixing the check to ensure that R_MethodsNamespace
> is not R_GlobalEnv, but one could load a serialized S4 object and
> expect S3 dispatch to work with Rscript.
>
> On Tue, Apr 19, 2016 at 6:51 AM, Gabriel Becker <gmbecker at
ucdavis.edu>
> wrote:
> > See also .isMethodsDispatchOn, which is what trace uses to decide if
the
> > methods package needs to be loaded.
> >
> > ~G
> >
> > On Tue, Apr 19, 2016 at 5:34 AM, Michael Lawrence
> > <lawrence.michael at gene.com> wrote:
> >>
> >> Not sure why R_has_methods_attached() exists. Maybe Martin could
shed
> >> some light on that.
> >>
> >> On Mon, Apr 18, 2016 at 11:50 PM, Kirill M?ller
> >> <kirill.mueller at ivt.baug.ethz.ch> wrote:
> >> > Thanks for looking into it, your approach sounds good to me.
See also
> >> > R_has_methods_attached()
> >> >
> >> > (
>
https://github.com/wch/r-source/blob/42ecf5f492a005f5398cbb4c9becd4aa5af9d05c/src/main/objects.c#L258-L265
> ).
> >> >
> >> > I'm fine with Rscript not loading "methods", as
long as everything
> works
> >> > properly with "methods" loaded but not attached.
> >> >
> >> >
> >> > -Kirill
> >> >
> >> >
> >> >
> >> > On 19.04.2016 04:10, Michael Lawrence wrote:
> >> >>
> >> >> Right, the methods package is not attached by default
when running R
> >> >> with Rscript. We should probably remove that special
case, as it
> >> >> mostly just leads to confusion, but that won't happen
immediately.
> >> >>
> >> >> For now, the S4_extends() should probably throw an error
when the
> >> >> methods namespace is not loaded. And the check should be
changed to
> >> >> directly check whether R_MethodsNamespace has been set to
something
> >> >> other than the default (R_GlobalEnv). Agreed?
> >> >>
> >> >> On Mon, Apr 18, 2016 at 4:35 PM, Kirill M?ller
> >> >> <kirill.mueller at ivt.baug.ethz.ch> wrote:
> >> >>>
> >> >>> Scenario: An S3 method is declared for an S4 base
class but called
> for
> >> >>> an
> >> >>> instance of a derived class.
> >> >>>
> >> >>> Steps to reproduce:
> >> >>>
> >> >>>> Rscript -e "test <- function(x)
UseMethod('test', x); test.Matrix
> <-
> >> >>>> function(x) 'Hi';
MatrixDispatchTest::test(Matrix::Matrix())"
> >> >>>
> >> >>> Error in UseMethod("test", x) :
> >> >>>    no applicable method for 'test' applied to
an object of class
> >> >>> "lsyMatrix"
> >> >>> Calls: <Anonymous>
> >> >>> 1: MatrixDispatchTest::test(Matrix::Matrix())
> >> >>>
> >> >>>> Rscript -e "extends <- 42; test <-
function(x) UseMethod('test',
> x);
> >> >>>> test.Matrix <- function(x) 'Hi';
> >> >>>> MatrixDispatchTest::test(Matrix::Matrix())"
> >> >>>
> >> >>> [1] "Hi"
> >> >>>
> >> >>> To me, it looks like a sanity check in line 655 of
src/main/attrib.c
> >> >>> is
> >> >>> making wrong assumptions, but there might be other
reasons.
> >> >>>
> >> >>>
> >> >>> (
>
https://github.com/wch/r-source/blob/780021752eb83a71e2198019acf069ba8741103b/src/main/attrib.c#L655-L656
> )
> >> >>>
> >> >>> Same behavior in R 3.2.4, R 3.2.5 and R-devel r70420.
> >> >>>
> >> >>>
> >> >>> Best regards
> >> >>>
> >> >>> Kirill
> >> >>>
> >> >>> ______________________________________________
> >> >>> R-devel at r-project.org mailing list
> >> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >> >>>
> >> >
> >> >
> >>
> >> ______________________________________________
> >> R-devel at r-project.org mailing list
> >> https://stat.ethz.ch/mailman/listinfo/r-devel
> >
> >
> >
> >
> > --
> > Gabriel Becker, PhD
> > Associate Scientist (Bioinformatics)
> > Genentech Research
>


-- 
Gabriel Becker, PhD
Associate Scientist (Bioinformatics)
Genentech Research

	[[alternative HTML version deleted]]