Bug 17434 - input of attributes() defined as "obj" but function expecting "x" instead
Summary: input of attributes() defined as "obj" but function expecting "x" instead
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: Language (show other bugs)
Version: R 3.5.0
Hardware: All All
: P5 minor
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2018-06-11 13:04 UTC by Alexandre Courtiol
Modified: 2018-06-19 07:50 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description 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
Comment 1 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
Comment 2 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.
Comment 3 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).
Comment 4 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?
Comment 5 Benjamin Tyner 2018-06-18 00:39:35 UTC
Perhaps it could be enhanced to accept either one? ("x" or "obj")
Comment 6 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.
Comment 7 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