Thanks to all who commented.   In some defense of the person who reported the 
"bug", it 
appeared to be a company policy from above, influenced by the fact that they 
had been 
often burned by not using :: when multiple packages use the same symbol.

Some further technical detail:  coxph has 3 specials: strata, cluster, and tt.  
For the 
last of these I took a different route, and did not make it an exported symbol 
in the 
survival package. (I try to be smarter over time).  Instead, coxph first looks 
at the 
formula.  If that formula contains tt() then it creates an envionment that 
contains the 
function  "tt <- funciton(x) x", whose parent is the environment of the 
formula, changes 
the formula's environment to this, and then proceeds as usual.    As a 
curiousity, I'm 
going to change cluster to the same form, and them run my script that executes 
R CMD  
check on every package that has surivval as depend/import/suggest and see how 
many 
break.   I'm guessing I'll find several that used cluster for their own ends. 
The strata 
function will be worse.

Duncan -- to use your approach, I'd instead hack the formula before calling 
model.frame, 
so that it does not try to call cluster?

Another approach I tried was not exporting cluster(), but that fails when 
model.frame 
tries to call it.


Terry


On 2/24/20 12:21 PM, Duncan Murdoch wrote:
> On 24/02/2020 8:55 a.m., Therneau, Terry M., Ph.D. via R-devel wrote:
>> I recently had a long argument wrt the survival package, namely that the 
>> following code
>> didn't do what they expected, and so they reported it as a bug
>>
>>     survival::coxph( survival::Surv(time, status) ~ age + sex + 
>> survival::strata(inst),
>> data=lung)
>>
>> a. The Google R style guide  recommends that one put :: everywhere
>> b. This breaks the recognition of cluster as a "special" in the terms 
>> function.
>>
>> I've been stubborn and said that their misunderstanding of how formulas work 
>> is not my
>> problem.   But I'm sure that the issue will come up again, and multiple 
>> other packages
>> will break.
>>
>> A big problem is that the code runs, it just gives the wrong answer.
>>
>> Suggestions?
>
> I don't know how widely used survival::strata is versus the special strata 
> (or cluster, 
> or other specials).  If you were just introducing this now, I'd try to make 
> sure that 
> only one of those worked:  don't have any functions matching the names of 
> specials, or 
> have functions that generate an error if you call them.  I did that in the 
> much less 
> widely used "tables" package, e.g. Heading() has special interpretation, and 
> the Heading 
> function is defined as
>
> Heading <- function(name = NULL, override = TRUE,
>                     character.only = FALSE,
>             nearData = TRUE)
>     stop("This is a pseudo-function, not meant to be called.")
>
> However, survival has far more users than tables does, so changing the name 
> of your 
> special functions or the corresponding regular functions could be a huge 
> headache.
>
> Perhaps there's a way to set a flag before evaluating the function in the 
> formula, and 
> generate a warning if survival::strata is called when it looks like the 
> special function 
> is intended.
>
> Duncan Murdoch 


        [[alternative HTML version deleted]]

______________________________________________
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

Reply via email to