S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

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

S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Kirill Müller
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

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Kirill Müller
Please omit "MatrixDispatchTest::" from the test scripts:

Rscript -e "test <- function(x) UseMethod('test', x); test.Matrix <-
function(x) 'Hi'; test(Matrix::Matrix())"

Rscript -e "extends <- 42; test <- function(x) UseMethod('test', x);
test.Matrix <- function(x) 'Hi'; test(Matrix::Matrix())"


-Kirill


On 19.04.2016 01:35, Kirill Müller 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
>
> ______________________________________________
> [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: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Michael Lawrence-3
In reply to this post by Kirill Müller
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
<[hidden email]> 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
>
> ______________________________________________
> [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: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Kirill Müller
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
> <[hidden email]> 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
>>
>> ______________________________________________
>> [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: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Michael Lawrence-3
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
<[hidden email]> 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
>> <[hidden email]> 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
>>>
>>> ______________________________________________
>>> [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: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Gabriel Becker
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 <[hidden email]
> 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
> <[hidden email]> 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
> >> <[hidden email]> 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
> >>>
> >>> ______________________________________________
> >>> [hidden email] mailing list
> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >>>
> >
> >
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>



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

        [[alternative HTML version deleted]]

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Michael Lawrence-3
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 <[hidden email]> 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
> <[hidden email]> 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
>> <[hidden email]> 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
>> >> <[hidden email]> 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
>> >>>
>> >>> ______________________________________________
>> >>> [hidden email] mailing list
>> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> >>>
>> >
>> >
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>
>
>
> --
> Gabriel Becker, PhD
> Associate Scientist (Bioinformatics)
> Genentech Research

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Gabriel Becker
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 <[hidden email]
> 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 <[hidden email]>
> 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
> > <[hidden email]> 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
> >> <[hidden email]> 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
> >> >> <[hidden email]> 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
> >> >>>
> >> >>> ______________________________________________
> >> >>> [hidden email] mailing list
> >> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >> >>>
> >> >
> >> >
> >>
> >> ______________________________________________
> >> [hidden email] 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]]

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

hadley wickham
This might be too big a change - but is it worth reconsidering the
behaviour of Rscript? Maybe the simplest fix would be simply to always
load the methods package.  (I think historically it didn't because
loading methods took a long time, but that is no longer true)

Hadley

On Tue, Apr 19, 2016 at 10:37 AM, Gabriel Becker <[hidden email]> wrote:

> 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 <[hidden email]
>> 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 <[hidden email]>
>> 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
>> > <[hidden email]> 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
>> >> <[hidden email]> 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
>> >> >> <[hidden email]> 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
>> >> >>>
>> >> >>> ______________________________________________
>> >> >>> [hidden email] mailing list
>> >> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> >> >>>
>> >> >
>> >> >
>> >>
>> >> ______________________________________________
>> >> [hidden email] 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]]
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel



--
http://hadley.nz

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Henrik Bengtsson-5
On Tue, Apr 19, 2016 at 9:21 AM, Hadley Wickham <[hidden email]> wrote:
>
> This might be too big a change - but is it worth reconsidering the
> behaviour of Rscript? Maybe the simplest fix would be simply to always
> load the methods package.  (I think historically it didn't because
> loading methods took a long time, but that is no longer true)

Slightly weaker version of this wish (that would also remove
confusion): At least make R and Rscript load the same set of packages
by default.


More clarification (in case some is new to this topic):

The packages loaded by default when R and Rscript is loaded can be
controlled by environment variable 'R_DEFAULT_PACKAGES' and/or option
'defaultPackages', cf. help("Startup").  When this is empty or
undefined, the built-in defaults kick in, and it's these built-in
defaults that differ between the R and the Rscript executable:

$ R --quiet --vanilla -e "getOption('defaultPackages')"
> getOption('defaultPackages')
[1] "datasets"  "utils"     "grDevices" "graphics"  "stats"     "methods"

$ Rscript --vanilla -e "getOption('defaultPackages')"
[1] "datasets"  "utils"     "grDevices" "graphics"  "stats"

Thus, a user can enforce the same set of default packages by using:

$ export R_DEFAULT_PACKAGES=datasets,utils,grDevices,graphics,stats,methods

$ R --quiet --vanilla -e "getOption('defaultPackages')"
> getOption('defaultPackages')
[1] "datasets"  "utils"     "grDevices" "graphics"  "stats"     "methods"

$ Rscript --vanilla -e "getOption('defaultPackages')"
[1] "datasets"  "utils"     "grDevices" "graphics"  "stats"     "methods"

/Henrik

>
> Hadley
>
> On Tue, Apr 19, 2016 at 10:37 AM, Gabriel Becker <[hidden email]> wrote:
> > 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 <[hidden email]
> >> 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 <[hidden email]>
> >> 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
> >> > <[hidden email]> 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
> >> >> <[hidden email]> 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
> >> >> >> <[hidden email]> 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
> >> >> >>>
> >> >> >>> ______________________________________________
> >> >> >>> [hidden email] mailing list
> >> >> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
> >> >> >>>
> >> >> >
> >> >> >
> >> >>
> >> >> ______________________________________________
> >> >> [hidden email] 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]]
> >
> > ______________________________________________
> > [hidden email] mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
>
>
>
> --
> http://hadley.nz
>
> ______________________________________________
> [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: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Dirk Eddelbuettel
In reply to this post by hadley wickham

On 19 April 2016 at 11:21, Hadley Wickham wrote:
| This might be too big a change - but is it worth reconsidering the
| behaviour of Rscript? Maybe the simplest fix would be simply to always
| load the methods package.  (I think historically it didn't because
| loading methods took a long time, but that is no longer true)

FWIW littler has always loaded package 'methods' at startup (because I found
this Rscript 'feature' to be too insufferable) -- and of course still starts
in about half the time as Rscript.

Dirk

--
http://dirk.eddelbuettel.com | @eddelbuettel | [hidden email]

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Michael Lawrence-3
In reply to this post by Gabriel Becker
Agreed about Rscript being consistent R.

For now, I'll modify S4_extends() so that it leads to S4 dispatch when
dispatch is turned on (not just when methods is attached).

On Tue, Apr 19, 2016 at 8:37 AM, Gabriel Becker <[hidden email]> wrote:

> 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
> <[hidden email]> 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 <[hidden email]>
>> 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
>> > <[hidden email]> 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
>> >> <[hidden email]> 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
>> >> >> <[hidden email]> 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
>> >> >>>
>> >> >>> ______________________________________________
>> >> >>> [hidden email] mailing list
>> >> >>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> >> >>>
>> >> >
>> >> >
>> >>
>> >> ______________________________________________
>> >> [hidden email] 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

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

Re: S3 dispatch for S4 subclasses only works if variable "extends" is accessible from global environment

Martin Maechler
In reply to this post by Michael Lawrence-3
>>>>> Michael Lawrence <[hidden email]>
>>>>>     on Tue, 19 Apr 2016 05:34:41 -0700 writes:

    > Not sure why R_has_methods_attached() exists. Maybe Martin could shed
    > some light on that.

It was to support (via 'classgets' in attrib.c) a very fast

   class(.) <- "<newclass>"

for S4 objects... to be used in  setAs(......)  methods,
e.g., for the Matrix package where you have many classes with
most slots the same, and I would have wanted to be clearly
faster than calling
   new("<newclass>", x=.., y=.., z=.., u=..,v=.. ..)

But that experiment has not finalized, maybe because it was a too hackish idea.

I may have started writing that at a time where we still mostly
thought that it was not possible to have a working S4 R
"environment" where methods was only loaded but not attached.

In conclusion, you can consider R_has_methods_attached as non-existent.


    > On Mon, Apr 18, 2016 at 11:50 PM, Kirill Müller
    > <[hidden email]> 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
    >>> <[hidden email]> 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
    >>>>
    >>>> ______________________________________________
    >>>> [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