R devel - Oct 2023 - dim<-() changed in R-devel; no longer removing "dimnames" when doing dim(x) <- dim(x)

>>>>> Henrik Bengtsson 
>>>>>     on Sun, 29 Oct 2023 10:42:19 -0700 writes:
    > Hello,

    > the fix of PR18612
    > (https://bugs.r-project.org/show_bug.cgi?id=18612) in
    > r85380
    >
(https://github.com/wch/r-source/commit/2653cc6203fce4c48874111c75bbccac3ac4e803)
    > caused a change in `dim<-()`.  Specifically, in the past,
    > any `dim<-()` assignment would _always_ remove "dimnames"
    > and "names" attributes per help("dim"):


    >     The replacement method changes the "dim" attribute
    > (provided the new value is compatible) and removes any
    > "dimnames" and "names" attributes.

    > In the new version, assigning the same "dim" as before
    > will no longer remove "dimnames".  I'm reporting here to
    > check whether this change was intended, or if it was an
    > unintended side effect of the bug fix.

    > For example, in R Under development (unstable) (2023-10-21
    > r85379), we would get:

    >> x <- array(1:2, dim=c(1,2), dimnames=list("A",
    >> c("a","b"))) str(dimnames(x))
    > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"

    >> dim(x) <- dim(x) ## Removes "dimnames" no matter what
    >> str(dimnames(x))
    >  NULL


    > whereas in R Under development (unstable) (2023-10-21
    > r85380) and beyond, we now get:

    >> x <- array(1:2, dim=c(1,2), dimnames=list("A",
    >> c("a","b"))) str(dimnames(x))
    > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"

    >> dim(x) <- dim(x) ## No longer removes "dimnames"
    >> str(dimnames(x))
    > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"

    >> dim(x) <- rev(dim(x)) ## Still removes "dimnames"
    >> str(dimnames(x))
    >  NULL

    > /Henrik

Thank you, Henrik.

This is "funny" (in an unusal sense):
indeed, the change was *in*advertent, by me (svn rev 85380).

I had experimentally {i.e., only in my own private version of R-devel!}
modified the behavior of `dim<-` somewhat 
such it does *not* unnecessarily drop dimnames,
e.g., in your   `dim(x) <- dim(x)` case above,
one could really argue that it's a "true loss" if x loses
dimnames "unnecessarily" ... 

OTOH, I knew in the mean time that  `dim<-` has always been
documented to drop dimnames in all cases,  and even more
importantly, I got a strong recommendation to *not* go further
with this idea -- not only for back compatibility reasons, but
also for internal logical consistency.

Most probably, we will just revert this inadvertent change,
but before that ... since it has been out in the wild anyway,
we could quickly consider if it *did* break code.

I assume it did, or you would not have noticed ?

Martin

Hi Martin, Henrik,

I actually like this change.

Makes a lot of sense IMO that dim(x) <- dim(x) be a no-op, or, more 
generally, that foo(x) <- foo(x) be a no-op for any setter/getter combo.

FWIW S4Arrays::set_dim() does that too. It also preserves the dimnames 
if the right value is only adding or dropping outermost (ineffective) 
dimensions:

 ??? > x <- array(1:6, dim=c(2,3,1), dimnames=list(c("A",
"B"),
c("x","y", "z"), "T"))
 ??? > S4Arrays:::set_dim(x, 2:3)
 ? ? ? x y z
 ??? A 1 3 5
 ??? B 2 4 6

Note that this is consistent with drop().

Best,

H.

On 10/30/23 03:53, Martin Maechler wrote:
>>>>>> Henrik Bengtsson
>>>>>>      on Sun, 29 Oct 2023 10:42:19 -0700 writes:
>      > Hello,
>
>      > the fix of PR18612
>      > (https://bugs.r-project.org/show_bug.cgi?id=18612) in
>      > r85380
>      >
(https://github.com/wch/r-source/commit/2653cc6203fce4c48874111c75bbccac3ac4e803)
>      > caused a change in `dim<-()`.  Specifically, in the past,
>      > any `dim<-()` assignment would _always_ remove
"dimnames"
>      > and "names" attributes per help("dim"):
>
>
>      >     The replacement method changes the "dim" attribute
>      > (provided the new value is compatible) and removes any
>      > "dimnames" and "names" attributes.
>
>      > In the new version, assigning the same "dim" as before
>      > will no longer remove "dimnames".  I'm reporting
here to
>      > check whether this change was intended, or if it was an
>      > unintended side effect of the bug fix.
>
>      > For example, in R Under development (unstable) (2023-10-21
>      > r85379), we would get:
>
>      >> x <- array(1:2, dim=c(1,2), dimnames=list("A",
>      >> c("a","b"))) str(dimnames(x))
>      > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"
>
>      >> dim(x) <- dim(x) ## Removes "dimnames" no matter
what
>      >> str(dimnames(x))
>      >  NULL
>
>
>      > whereas in R Under development (unstable) (2023-10-21
>      > r85380) and beyond, we now get:
>
>      >> x <- array(1:2, dim=c(1,2), dimnames=list("A",
>      >> c("a","b"))) str(dimnames(x))
>      > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"
>
>      >> dim(x) <- dim(x) ## No longer removes "dimnames"
>      >> str(dimnames(x))
>      > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"
>
>      >> dim(x) <- rev(dim(x)) ## Still removes
"dimnames"
>      >> str(dimnames(x))
>      >  NULL
>
>      > /Henrik
>
> Thank you, Henrik.
>
> This is "funny" (in an unusal sense):
> indeed, the change was *in*advertent, by me (svn rev 85380).
>
> I had experimentally {i.e., only in my own private version of R-devel!}
> modified the behavior of `dim<-` somewhat
> such it does *not* unnecessarily drop dimnames,
> e.g., in your   `dim(x) <- dim(x)` case above,
> one could really argue that it's a "true loss" if x loses
> dimnames "unnecessarily" ...
>
> OTOH, I knew in the mean time that  `dim<-` has always been
> documented to drop dimnames in all cases,  and even more
> importantly, I got a strong recommendation to *not* go further
> with this idea -- not only for back compatibility reasons, but
> also for internal logical consistency.
>
> Most probably, we will just revert this inadvertent change,
> but before that ... since it has been out in the wild anyway,
> we could quickly consider if it *did* break code.
>
> I assume it did, or you would not have noticed ?
>
> Martin
>
> ______________________________________________
> R-devel at r-project.org  mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
-- 
Herv? Pag?s

Bioconductor Core Team
hpages.on.github at gmail.com

	[[alternative HTML version deleted]]

> I assume it did, or you would not have noticed ?
I noticed it because I got a notice from CRAN about 'matrixStats'
starting to fail on R-devel.  It was a non-critical failure, because
it was due to how the package tests compare the results to the
corresponding base-R implementation. Basically, for legacy reasons
there was a `dim(res) <- dim` statement and anything following assumed
the "dimnames" would be done. I've since rewritten the tests to
not
make such assumptions, which resulted in code that is easier to
follow. So, there were good outcomes from this change too :)

The discussion on whether certain R expressions (e.g. dim(x) <-
dim(x)) should be no-op is interesting, but it's much bigger, and I
can see how it becomes a quite complicated discussion.

Thanks,

Henrik


On Mon, Oct 30, 2023 at 3:53?AM Martin Maechler
<maechler at stat.math.ethz.ch> wrote:
>
>
> >>>>> Henrik Bengtsson
> >>>>>     on Sun, 29 Oct 2023 10:42:19 -0700 writes:
>
>     > Hello,
>
>     > the fix of PR18612
>     > (https://bugs.r-project.org/show_bug.cgi?id=18612) in
>     > r85380
>     >
(https://github.com/wch/r-source/commit/2653cc6203fce4c48874111c75bbccac3ac4e803)
>     > caused a change in `dim<-()`.  Specifically, in the past,
>     > any `dim<-()` assignment would _always_ remove
"dimnames"
>     > and "names" attributes per help("dim"):
>
>
>     >     The replacement method changes the "dim" attribute
>     > (provided the new value is compatible) and removes any
>     > "dimnames" and "names" attributes.
>
>     > In the new version, assigning the same "dim" as before
>     > will no longer remove "dimnames".  I'm reporting
here to
>     > check whether this change was intended, or if it was an
>     > unintended side effect of the bug fix.
>
>     > For example, in R Under development (unstable) (2023-10-21
>     > r85379), we would get:
>
>     >> x <- array(1:2, dim=c(1,2), dimnames=list("A",
>     >> c("a","b"))) str(dimnames(x))
>     > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"
>
>     >> dim(x) <- dim(x) ## Removes "dimnames" no matter
what
>     >> str(dimnames(x))
>     >  NULL
>
>
>     > whereas in R Under development (unstable) (2023-10-21
>     > r85380) and beyond, we now get:
>
>     >> x <- array(1:2, dim=c(1,2), dimnames=list("A",
>     >> c("a","b"))) str(dimnames(x))
>     > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"
>
>     >> dim(x) <- dim(x) ## No longer removes "dimnames"
>     >> str(dimnames(x))
>     > List of 2 $ : chr "A" $ : chr [1:2] "a"
"b"
>
>     >> dim(x) <- rev(dim(x)) ## Still removes "dimnames"
>     >> str(dimnames(x))
>     >  NULL
>
>     > /Henrik
>
> Thank you, Henrik.
>
> This is "funny" (in an unusal sense):
> indeed, the change was *in*advertent, by me (svn rev 85380).
>
> I had experimentally {i.e., only in my own private version of R-devel!}
> modified the behavior of `dim<-` somewhat
> such it does *not* unnecessarily drop dimnames,
> e.g., in your   `dim(x) <- dim(x)` case above,
> one could really argue that it's a "true loss" if x loses
> dimnames "unnecessarily" ...
>
> OTOH, I knew in the mean time that  `dim<-` has always been
> documented to drop dimnames in all cases,  and even more
> importantly, I got a strong recommendation to *not* go further
> with this idea -- not only for back compatibility reasons, but
> also for internal logical consistency.
>
> Most probably, we will just revert this inadvertent change,
> but before that ... since it has been out in the wild anyway,
> we could quickly consider if it *did* break code.
>
> I assume it did, or you would not have noticed ?
>
> Martin