Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars

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

Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars

R devel mailing list
SVN revision 77401 changes
        x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[class(o) != "AsIs"]))
to
        x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")]))
in function 'get_all_vars' in src/library/stats/R/models.R in R devel.

The change is inappropriate.

class(o)[class(o) != "AsIs"] removes "AsIs" from class(o), giving class(o) without "AsIs".

On the other hand, inherits(o,"AsIs") is just a single logical value. If "AsIs" is in class(o), inherits(o,"AsIs") is TRUE. In that case, by recycling of logical index, class(o)[!inherits(o,"AsIs")] removes all elements of class(o), giving character(0).

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

Re: Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars

Martin Maechler
>>>>> suharto anggono--- via R-devel
>>>>>     on Sun, 17 Nov 2019 10:34:31 +0000 writes:

    > SVN revision 77401 changes
    >         x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[class(o) != "AsIs"]))
    > to
    >         x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")]))
    > in function 'get_all_vars' in src/library/stats/R/models.R in R devel.

    > The change is inappropriate.

    > class(o)[class(o) != "AsIs"] removes "AsIs" from class(o), giving class(o) without "AsIs".

    > On the other hand, inherits(o,"AsIs") is just a single logical value. If "AsIs" is in class(o), inherits(o,"AsIs") is TRUE. In that case, by recycling of logical index, class(o)[!inherits(o,"AsIs")] removes all elements of class(o), giving character(0).

Thank you, Suharto !

You are obviously right,  and I'm a bit embarrassed by my
overzealousness to follow my own recommendations in the  R Blog

  http://bit.ly/R_blog_class_think_2x

{*wrongly*: The recommendation was to "think again" ...}

It's a "shame" that the wrong code did not trigger any checks,
so if anybody has time... I'd be grateful for such a regression
check.

Martin

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

Re: Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars

Martin Maechler
>>>>> Martin Maechler
>>>>>     on Mon, 18 Nov 2019 12:15:38 +0100 writes:

>>>>> suharto anggono--- via R-devel
>>>>>     on Sun, 17 Nov 2019 10:34:31 +0000 writes:

    >> SVN revision 77401 changes
    >> x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[class(o) != "AsIs"]))
    >> to
    >> x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")]))
    >> in function 'get_all_vars' in src/library/stats/R/models.R in R devel.

    >> The change is inappropriate.

    >> class(o)[class(o) != "AsIs"] removes "AsIs" from class(o), giving class(o) without "AsIs".

    >> On the other hand, inherits(o,"AsIs") is just a single logical value. If "AsIs" is in class(o), inherits(o,"AsIs") is TRUE. In that case, by recycling of logical index, class(o)[!inherits(o,"AsIs")] removes all elements of class(o), giving character(0).

    > Thank you, Suharto !

    > You are obviously right,  and I'm a bit embarrassed by my
    > overzealousness to follow my own recommendations in the  R Blog

    > http://bit.ly/R_blog_class_think_2x

    > {*wrongly*: The recommendation was to "think again" ...}

    > It's a "shame" that the wrong code did not trigger any checks,
    > so if anybody has time... I'd be grateful for such a regression
    > check.

Once I started thinking, it was easy to modify the previous
reg.check  to trigger in the case of the erronous r77401.

Fixed now in 77435.
Martin

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

Re: Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars

R devel mailing list
 
class(o)[!inherits(o,"AsIs")] is still in function 'get_all_vars' in R patched (in https://svn.r-project.org/R/branches/R-3-6-branch/src/library/stats/R/models.R). It was ported to R patched by r77402.     On Monday, 18 November 2019, 8:12:10 PM GMT+7, Martin Maechler <[hidden email]> wrote:  
 
 >>>>> Martin Maechler
>>>>>    on Mon, 18 Nov 2019 12:15:38 +0100 writes:

>>>>> suharto anggono--- via R-devel
>>>>>    on Sun, 17 Nov 2019 10:34:31 +0000 writes:

    >> SVN revision 77401 changes
    >> x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[class(o) != "AsIs"]))
    >> to
    >> x[isM] <- lapply(x[isM], function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")]))
    >> in function 'get_all_vars' in src/library/stats/R/models.R in R devel.

    >> The change is inappropriate.

    >> class(o)[class(o) != "AsIs"] removes "AsIs" from class(o), giving class(o) without "AsIs".

    >> On the other hand, inherits(o,"AsIs") is just a single logical value. If "AsIs" is in class(o), inherits(o,"AsIs") is TRUE. In that case, by recycling of logical index, class(o)[!inherits(o,"AsIs")] removes all elements of class(o), giving character(0).

    > Thank you, Suharto !

    > You are obviously right,  and I'm a bit embarrassed by my
    > overzealousness to follow my own recommendations in the  R Blog

    > http://bit.ly/R_blog_class_think_2x

    > {*wrongly*: The recommendation was to "think again" ...}

    > It's a "shame" that the wrong code did not trigger any checks,
    > so if anybody has time... I'd be grateful for such a regression
    > check.

Once I started thinking, it was easy to modify the previous
reg.check  to trigger in the case of the erronous r77401.

Fixed now in 77435.
Martin
 
        [[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: Inappropriate class(o)[!inherits(o,"AsIs")] in get_all_vars

Martin Maechler
>>>>> suharto anggono--- via R-devel
>>>>>     on Fri, 29 Nov 2019 18:05:34 +0000 writes:

    > class(o)[!inherits(o,"AsIs")] is still in function
    > 'get_all_vars' in R patched (in
    > https://svn.r-project.org/R/branches/R-3-6-branch/src/library/stats/R/models.R). It
    > was ported to R patched by r77402.

You are right.... it's no longer now,  thank you very much,
Suharto!

Martin


    > On Monday, 18 November 2019, 8:12:10 PM GMT+7, Martin Maechler
    > <[hidden email]> wrote:
 
    >>>>>> Martin Maechler     on Mon, 18 Nov 2019 12:15:38
    >>>>>> +0100 writes:

>>>>> suharto anggono--- via R-devel
    >>>>>>     on Sun, 17 Nov 2019 10:34:31 +0000 writes:

    >     >> SVN revision 77401 changes     >> x[isM] <-
    > lapply(x[isM], function(o) `class<-`(o, class(o)[class(o)
    > != "AsIs"]))     >> to     >> x[isM] <- lapply(x[isM],
    > function(o) `class<-`(o, class(o)[!inherits(o,"AsIs")]))  
    >   >> in function 'get_all_vars' in
    > src/library/stats/R/models.R in R devel.

    >     >> The change is inappropriate.

    >     >> class(o)[class(o) != "AsIs"] removes "AsIs" from
    > class(o), giving class(o) without "AsIs".

    >     >> On the other hand, inherits(o,"AsIs") is just a
    > single logical value. If "AsIs" is in class(o),
    > inherits(o,"AsIs") is TRUE. In that case, by recycling of
    > logical index, class(o)[!inherits(o,"AsIs")] removes all
    > elements of class(o), giving character(0).

    >     > Thank you, Suharto !

    >     > You are obviously right,  and I'm a bit embarrassed
    > by my     > overzealousness to follow my own
    > recommendations in the  R Blog

    >     > http://bit.ly/R_blog_class_think_2x

    >     > {*wrongly*: The recommendation was to "think again"
    > ...}

    >     > It's a "shame" that the wrong code did not trigger
    > any checks,     > so if anybody has time... I'd be
    > grateful for such a regression     > check.

    > Once I started thinking, it was easy to modify the
    > previous reg.check  to trigger in the case of the erronous
    > r77401.

    > Fixed now in 77435.  Martin
 
    > [[alternative HTML version deleted]]

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

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