R devel - Jun 2017 - [bug] droplevels() also drop object attributes (comment…)

Hi,

Just reporting a small bug? not really a big deal, but I don?t think that is
intended: droplevels()?also drops all object?s attributes.

Example:
> > test <- c("hello", "something", "hi")
> > test <- factor(test)
> > comment(test) <- "this is a test"
> > attr(test, "description") <- "this is another
test"
> > attributes(test)
> $levels
> [1] "hello" ? ? "hi" ? ? ? ?"something"
>
> $class
> [1] "factor"
>
> $comment
> [1] "this is a test"
>
> $description
> [1] "this is another test"
>
> > test <- droplevels(test)
> > attributes(test)
> $levels
> [1] "hello" ? ? "hi" ? ? ? ?"something"
>
> $class
> [1] "factor"

Serge

	[[alternative HTML version deleted]]

>>>>> Serge Bibauw <sbibauw at gmail.com>
>>>>>     on Mon, 15 May 2017 11:59:32 -0400 writes:
    > Hi,

    > Just reporting a small bug? not really a big deal, but I don?t think
that is intended: droplevels()?also drops all object?s attributes.

Yes.  The help page for droplevels (or the simple definition of
'droplevels.factor') clearly indicate that the method for
factors is really just a call to   factor(x, exclude = *)

and that _is_ quite an important base function whose semantic
should not be changed lightly. Still, let's continue :

Looking a bit, I see that the current behavior of factor() {and
hence droplevels} has been unchanged in this respect  for the
whole history of R, well, at least for more than 17 years (R 1.0.1, April 2000).

I'd agree there _is_ a bug, at least in the documentation which
does *not* mention that currently, all attributes are dropped but
"names",
"levels" (and "class").

OTOH, factor() would only need a small change to make it
preserve all attributes (but "class" and "levels" which are
set explicitly).

I'm sure this will break some checks in some packages.
Is it worth it?

e.g., our own R  QC checks currently check (the printing of) the
following (in tests/reg-tests-2.R ):

    > ## some tests of factor matrices
    > A <- factor(7:12)
    > dim(A) <- c(2, 3)
    > A
	 [,1] [,2] [,3]
    [1,] 7    9    11  
    [2,] 8    10   12  
    Levels: 7 8 9 10 11 12
    > str(A)
     factor [1:2, 1:3] 7 8 9 10 ...
     - attr(*, "levels")= chr [1:6] "7" "8"
"9" "10" ...
    > A[, 1:2]
	 [,1] [,2]
    [1,] 7    9   
    [2,] 8    10  
    Levels: 7 8 9 10 11 12
    > A[, 1:2, drop=TRUE]
    [1] 7  8  9  10
    Levels: 7 8 9 10

with the proposed change to factor(),
the last call would change its result:

    > A[, 1:2, drop=TRUE]
	 [,1] [,2]
    [1,] 7    9   
    [2,] 8    10  
    Levels: 7 8 9 10

because 'drop=TRUE' calls factor(..) and that would also
preserve the "dim" attribute.
I would think that the changed behavior _is_ better, and is also
according to documentation, because the help page for
 [.factor
explains that 'drop = TRUE' drops levels, but _not_ that it
transforms a factor matrix into a factor (vector).


Martin


    > Example:

    >> > test <- c("hello", "something",
"hi")
    >> > test <- factor(test)
    >> > comment(test) <- "this is a test"
    >> > attr(test, "description") <- "this is
another test"
    >> > attributes(test)
    >> $levels
    >> [1] "hello" ? ? "hi" ? ? ?
?"something"
    >> 
    >> $class
    >> [1] "factor"
    >> 
    >> $comment
    >> [1] "this is a test"
    >> 
    >> $description
    >> [1] "this is another test"
    >> 
    >> > test <- droplevels(test)
    >> > attributes(test)
    >> $levels
    >> [1] "hello" ? ? "hi" ? ? ?
?"something"
    >> 
    >> $class
    >> [1] "factor"


    > Serge

>>>>> Martin Maechler <maechler at stat.math.ethz.ch>
>>>>>     on Tue, 16 May 2017 11:01:23 +0200 writes:
>>>>> Serge Bibauw <sbibauw at gmail.com>
>>>>>     on Mon, 15 May 2017 11:59:32 -0400 writes:
    >> Hi,

    >> Just reporting a small bug? not really a big deal, but I
    >> don?t think that is intended: droplevels()?also drops all
    >> object?s attributes.

    > Yes.  The help page for droplevels (or the simple
    > definition of 'droplevels.factor') clearly indicate that
    > the method for factors is really just a call to factor(x,
    > exclude = *)

    > and that _is_ quite an important base function whose
    > semantic should not be changed lightly. Still, let's
    > continue :

    > Looking a bit, I see that the current behavior of factor()
    > {and hence droplevels} has been unchanged in this respect
    > for the whole history of R, well, at least for more than
    > 17 years (R 1.0.1, April 2000).

    > I'd agree there _is_ a bug, at least in the documentation
    > which does *not* mention that currently, all attributes
    > are dropped but "names", "levels" (and
"class").

    > OTOH, factor() would only need a small change to make it
    > preserve all attributes (but "class" and "levels"
which
    > are set explicitly).

    > I'm sure this will break some checks in some packages.  Is
    > it worth it?
> e.g., our own R  QC checks currently check (the printing of) the
> following (in tests/reg-tests-2.R ):
> 	> ## some tests of factor matrices
> 	> A <- factor(7:12)
> 	> dim(A) <- c(2, 3)
> 	> A
> 	     [,1] [,2] [,3]
> 	[1,] 7    9    11  
> 	[2,] 8    10   12  
> 	Levels: 7 8 9 10 11 12
> 	> str(A)
> 	 factor [1:2, 1:3] 7 8 9 10 ...
> 	 - attr(*, "levels")= chr [1:6] "7" "8"
"9" "10" ...
> 	> A[, 1:2]
> 	     [,1] [,2]
> 	[1,] 7    9   
> 	[2,] 8    10  
> 	Levels: 7 8 9 10 11 12
> 	> A[, 1:2, drop=TRUE]
> 	[1] 7  8  9  10
> 	Levels: 7 8 9 10
> 
>     with the proposed change to factor(),
>     the last call would change its result:
> 
> 	> A[, 1:2, drop=TRUE]
> 	     [,1] [,2]
> 	[1,] 7    9   
> 	[2,] 8    10  
> 	Levels: 7 8 9 10
    > because 'drop=TRUE' calls factor(..) and that would also
    > preserve the "dim" attribute.  I would think that the
    > changed behavior _is_ better, and is also according to
    > documentation, because the help page for [.factor explains
    > that 'drop = TRUE' drops levels, but _not_ that it
    > transforms a factor matrix into a factor (vector).

    > Martin

I'm finally coming back to this.
It still seems to make sense to change factor() and hence
droplevels() behavior here, and plan to commit this change
within a day.

Martin Maechler
ETH Zurich