[Rd] factor() calls sort.list unnecessarily?

[Petr Savicky]

On Fri, Jul 03, 2009 at 07:57:49AM -0700, Martin Morgan wrote:
[...]
> sort.list is always called but used only to determine the order of
> levels, so unnecessary when levels are provided.

I think, this is correct. Replacing 
  ind <- sort.list(x)
by
  if (missing(levels))
      ind <- sort.list(x)
makes factor() more efficient, when levels parameter is not missing
and since variable ind is not needed in this case, i think, the
modification in the above form (without unique()) is correct.
Parameter levels is not used between the two tests of missing(levels),
so we get the same result in both cases as needed.

> In addition, order of
> levels is for unique values of x only. Perhaps these issues are
> addressed in the patch below? It does require unique() on the original
> argument x, rather than only on as.character(x)

Computing the order of levels is sufficient for unique levels, however,
if x is numeric, then the operation
  x <- as.character(x)
performs rounding, due to which different, but very close numbers 
are mapped to the same value. So, the length of unique(x) may change.

A possible solution could be to keep unique(x) for the original values
and refer to it, when constructing the levels. For example, as follows.
  y <- unique(x)
  ind <- sort.list(y)
  y <- as.character(y)
  levels <- unique(y[ind])
  x <- as.character(x)
  f <- match(x, levels)
This is more efficient, if the length of unique(x) is significantly
smaller then the length of x. On the other hand, if their lengths are
similar, then computing as.character() on both x and y incures some
slow down.

> At the least, perhaps
> sort.list can be called only when levels are not provided?

I support this part of the patch without unique().

Petr.