Bug 17434 - input of attributes() defined as "obj" but function expecting "x" instead
input of attributes() defined as "obj" but function expecting "x" instead
 Status: CLOSED FIXED None R Unclassified Language (show other bugs) R 3.5.0 All All P5 minor R-core

 Reported: 2018-06-11 13:04 UTC by Alexandre Courtiol 2018-06-19 07:50 UTC (History) 3 users (show) btyner maechler tomas.kalibera

Attachments

 Note You need to log in before you can comment on or make changes to this bug.
 Alexandre Courtiol 2018-06-11 13:04:38 UTC Dear R core team, I think I noticed a small issue in the definition of the function attributes(). The function attributes() is defined as expecting an input named 'obj': > attributes function (obj) .Primitive("attributes") Which is also what the *.Rmd implies: \usage{ attributes(obj) attributes(obj) <- value mostattributes(obj) <- value } \arguments{ \item{obj}{an object} \item{value}{an appropriate named list of attributes, or \code{NULL}.} } Most users don't rely on a named input, so they won't see the issue: > v <- c(a = 1) > attributes(v) $names [1] "a" But if we try to write things down properly, it does not work: > attributes(obj = v) Error in attributes(obj = v) : supplied argument name 'obj' does not match 'x' And (un)surprisingly the good input name seems to be 'x': > attributes(x = v)$names [1] "a" So I think that this is a bug but I cannot track the definition of the primitive in the sources to check what is really going on. Best, Alex Alexandre Courtiol 2018-06-12 07:10:01 UTC I have tried to track the issue, it seems to come from: r-source/src/main/attrib.c line 1221: check1arg(args, call, "x"); which should probably be: check1arg(args, call, "obj"); unless one prefers to change the initial function definition to keep "x". The error seems to have been introduced by Ripley commit "check arity and args of one-argument primitives" commited on Mar 14, 2010: 0ccaee6c4bd08009020cecf0bec2c929316314ee ++ Alex Martin Maechler 2018-06-12 08:10:37 UTC You are right. Checking the *names* of primitives was mostly introduced in svn c51267 | 2010-03-14 "check arity and args of one-argument primitives" introducing the check1arg() C utility, and this must have been a small lapsus in the many changes at the time. So we have a case of "wrong code" with correct documentation. I'd like to fix this in the hope that it won't break existing code... but that hope would almost surely be in vain. Tomas Kalibera 2018-06-13 13:16:12 UTC We could also update the documentation and .ArgsEnv, that should not break existing code. "x" is already used for argument name in class(), names(), attr(), so perhaps it would make the naming even more consistent (except mostattributes<-() which accepts also "obj", but where changing in principle could break existing code). Martin Maechler 2018-06-15 08:29:29 UTC (In reply to Tomas Kalibera from comment #3) > We could also update the documentation and .ArgsEnv, that should not break > existing code. "x" is already used for argument name in class(), names(), > attr(), so perhaps it would make the naming even more consistent (except > mostattributes<-() which accepts also "obj", but where changing in principle > could break existing code). yes, and also it would have 'obj' and 'x' "on the same page"... I see (in the blue book), that S actually did use 'x' for attributes attributes<- .. I did find other usages of attr1arg() in the C file which I adapted, but actually only by commenting them, as it is a no-op when the primitive function has more than one argument, which is e.g., the case for attributes<- Other opinions? Benjamin Tyner 2018-06-18 00:39:35 UTC Perhaps it could be enhanced to accept either one? ("x" or "obj") Martin Maechler 2018-06-18 16:13:18 UTC (In reply to Benjamin Tyner from comment #5) > Perhaps it could be enhanced to accept either one? ("x" or "obj"). You mean mostattributes<- of course... yeah, but the code to do that is somewhat ugly .. and I think Tomas' suggestion is where to go... and I plan to do the change for mostattributes as well. It's a relatively rarely used function and so I think chances are quite high of the change to triggering adversely. Martin Maechler 2018-06-19 07:50:24 UTC (In reply to Martin Maechler from comment #6) ..... about mostattributes<- ... > It's a relatively rarely used function and so I think chances are quite high > of the change to triggering adversely. a *NOT* was missing above: As mostattributes<- is rarely used, changes are high that the change will *NOT* have any bad effect. Change committed to R-devel, svn 74908