[Rd] [External] Re: Change to I() in R 4.1

[Pages, Herve]

On 10/30/20 10:48, luke-tierney using uiowa.edu wrote:
> On Fri, 30 Oct 2020, Pages, Herve wrote:
> 
>>
>> On 10/29/20 23:08, Pages, Herve wrote:
>> ...
>>>
>>> I can think of 2 ways to move forward:
>>>
>>> 1. Keep I()'s current implementation but suppress the warning. We'll
>>> make the necessary adjustments to DataFrame() to repair columns supplied
>>> as I(<S4>) objects. Note that we would still be in the situation where
>>> I(<S4>) objects break validObject() but we've been in that situation for
>>> years and so far we've managed to work around it. However this doesn't
>>> mean that validObject() shouldn't be fixed. Note that print(I(<S4>))
>>> would also need to be fixed (it says "<S4 Type Object>" which is
>>> misleading). Anyways, these 2 issues are separated from the main issue
>>> and can be dealt with later.
>>
>> 1b. A variant of the above could be to use the old implementation for S4
>> objects only:
>>
>>   I <- function(x)
>>   {
>>       if (isS4(x)) {
>>           structure(x, class = unique.default(c("AsIs", oldClass(x))))
>>       } else {
>>           `class<-`(x, unique.default(c("AsIs", oldClass(x))))
>>       }
>>   }
>>
>> That is probably a good compromise for now.
> 
> Not really. The underlying problem is that class<- and attributes<-
> (which is what structure() uses) handle the 'class' attribute
> differently, and that needs to be fixed. I don't have a strong opinion
> on what either should do, but they should do the same thing.

This is why I'm calling this a compromise. Many Bioconductor packages 
are broken right now so this would at least repair them and restore an 
important feature of the DataFrame() constructor, while at the same time 
preserve the speedup for all non-S4 objects that Martin was after when 
he changed I()'s implementation.

> 
> It's probably worth re-thinking the I() mechanism. ?Modifying the
> value, whether by changing the class or an attribute, is going to be
> brittle. A little less so for an attribute, but using an attribute
> rules out dispatch on the AsIs property.

Which was my 2nd proposal. It rules out dispatch but it's not clear why 
you would rely on dispatch for something like this.

Anyways, I personally have no preferences as long as I(<S4>)'s warning 
goes away and attr(class(<S4>), "package") can somehow be propagated.

Thanks,
H.

> 
> Best,
> 
> luke
> 
>>
>> I would also suggest that the "package" attribute of the S4 class be
>> kept around so the code that we use to restore the original object has a
>> way to restore it exactly, including its full class specification. Right
>> now, and also with the previous implementation, we cannot do that
>> because attr(class(x), "package") is lost. So something like this:
>>
>>   I <- function(x)
>>   {
>>       if (isS4(x)) {
>>           x_class <- class(x)
>>           new_classes <- c("AsIs", x_class)
>>           attr(new_classes, "package") <- attr(x_class, "package")
>>           structure(x, class=new_classes)
>>       } else {
>>           `class<-`(x, unique.default(c("AsIs", oldClass(x))))
>>       }
>>   }
>>
>> Thanks,
>> H.
>>
>>
>>
> 

-- 
Hervé Pagès

Program in Computational Biology
Division of Public Health Sciences
Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N, M1-B514
P.O. Box 19024
Seattle, WA 98109-1024

E-mail: hpages using fredhutch.org
Phone:  (206) 667-5791
Fax:    (206) 667-1319