Skip to content

Minimum of an ordered factor

8 messages · Thaler, Thorn, LAUSANNE, Applied Mathematics, Martin Maechler, Kurt Hornik

Original
Martin Maechler
# 
> Hi everybody, Is there a particular reason, why this code
    > does not work as intended:

    > z <- factor(LETTERS[1:3], ordered = TRUE)
    > u <- 4:6
    > min(z[u > 4])

    > Error in Summary.factor(2:3, na.rm = FALSE) :
    >   min not meaningful for factors



    > I agree that min is indeed not meaningful for not ordered
    > factors, but it makes sense for ordered
    > factors. Especially since

    > z[3] < z[2]
    > sort(z)

    > _ARE_ defined and work as expected. 

I agree that it is natural then, to expect  min(), max() and
range() to work as well.

    > Of course I can do something like

    > sort(z[u>4])[1]

    > but this does not enhance readability of my code. Thus, I
    > overloaded Summary.ordered as follows:


 

    > Summary.ordered <- function(..., na.rm) {

    >   ok <- switch(.Generic, max = , min = , range = TRUE,
    > FALSE)

    >   if (!ok) {

    >     warning(sprintf("'%s' is not meaningful for ordered
    > factors", .Generic))

    >     return(NA)

    >   }

    >   args <- list(...)

    >   level.list <- lapply(args, levels)

    >   level.set <- Reduce(union, level.list)

    >   if (!all(sapply(args, is.ordered)) ||
    > !all(sapply(level.list, identical, y = level.set))) {

    >     stop(sprintf("'%s' is only meaningful for ordered
    > factors if all arguments are ordered factors with the same
    > level sets",

    >                  .Generic))

    >   }

 

    >   codes <- lapply(args, as.integer)

    >   ind <- do.call(.Generic, c(codes, na.rm = na.rm))

 

    >   factor(level.set[ind], levels = level.set, ordered =
    > TRUE)

    > }

(the above is now even more garbled than it was already by your
 use of HTML-ified e-mail ..)

But your code is fine, even nice, in most parts,
and I will add (most of) it (and some documentation) to R-devel
 
unless we get contradicting comments :

    > Any comments appreciated.
(still)


Thank you, Thorn!

With regards,
Martin Maechler, ETH Zurich
Kurt Hornik
# 
I have 3 comments:
Same for me.
I think it would be better to use something like

  ll <- lapply(args, levels)

  !all(sapply(ll, identical, ll[[1L]]))

[using union() is not quite right]
The general comment is that if we support this I don't see why we should
not also support c.ordered (and in fact also c.factor) with the same
restrictions (identical level sequences for ordered and level sets for
factors).  We already have Ops.factor and Ops.ordered using the same
principle afaic.

If we add c.ordered, we should be able to encapsulate the identity of
levels testing into this, and simply use

    x <- c(...)

and then call .Generic on the codes of x etc.

Best
-k

        

      
      
    

  
  


  


      

    

    

      
      

        


  

        

      Thaler, Thorn, LAUSANNE, Applied Mathematics
    

    

      
      #
    

  


  

      
Kurt Hornik writes

            

      
      
      
    

        
Yes definitely. This line is in fact just a relic from a previous idea I
had.

            

      
      
      
    

        
Sounds reasonable. Ack.

BR Thorn

  
  


  


      

    

    

      
      

        


  

        

      Martin Maechler
    

    

      
      #
    

  


  

          

      
      
      
    

        
TTLAM> Kurt Hornik writes
    >> >> if (!all(sapply(args, is.ordered)) ||
    >> >> !all(sapply(level.list, identical, y = level.set))) {
    >> 
    >> I think it would be better to use something like
    >> 
    >> ll <- lapply(args, levels)
    >> 
    >> !all(sapply(ll, identical, ll[[1L]]))
    >> 
    >> [using union() is not quite right]

    TTLAM> Yes definitely. This line is in fact just a relic from a previous idea I
    TTLAM> had.
 
I have now committed the amended proposal (rev 53925);
thank you for the feedbacks..


    >> The general comment is that if we support this I don't see why we
    >> should
    >> not also support c.ordered (and in fact also c.factor) with the same
    >> restrictions (identical level sequences for ordered and level sets for
    >> factors).  We already have Ops.factor and Ops.ordered using the same
    >> principle afaic.

Yes, I think, too.

    >> If we add c.ordered, we should be able to encapsulate the identity of
    >> levels testing into this, and simply use
    >> 
    >> x <- c(...)
    >> 
    >> and then call .Generic on the codes of x etc.

    TTLAM> Sounds reasonable. Ack.

Yes, adding c.factor() and c.ordered() seems reasonable in
principle.
However, S and R now have a more than 20 year old history of
silently coercing factors to there integer codes with c(),
that I'm not yet sure we can do this without breaking too much
code [[and I am pretty sure this topic has been discusses before]].

I think we should start discussing the issue in a new thread
with proper Subject explicitly mention "c()" or "c.factor"/"c.ordered".
Martin



    TTLAM> BR Thorn

    TTLAM> ______________________________________________
    TTLAM> R-devel at r-project.org mailing list
    TTLAM> https://stat.ethz.ch/mailman/listinfo/r-devel

  
  


  


      

    

    

      
      

        


  

        

      Thaler, Thorn, LAUSANNE, Applied Mathematics
    

    

      
      #
    

  


  

          

      
      
      
    

        
I had a look at it and there is a typo:
	
stop(gettextf("'%s' not defined for \"difftime\" objects", .Generic),
domain = NA)

should rather be

stop(gettextf("'%s' not defined for ordered factors", .Generic), domain
= NA)

BR Thorn

  
  


  


      

    

    

      
      

        


  

        

      Martin Maechler
    

    

      
      #
    

  


  

          

      
      
      
    

        
>> Martin Maechler writes
    >> I have now committed the amended proposal (rev 53925);
    >> thank you for the feedbacks..

    > I had a look at it and there is a typo:
	
    > stop(gettextf("'%s' not defined for \"difftime\" objects", .Generic),
    > domain = NA)

    > should rather be

    > stop(gettextf("'%s' not defined for ordered factors", .Generic), domain
    TM> = NA)

aah.. blushshsh ! .... (and thanks).
Martin

  
  


  


      

    

    

      
      

        


  

        

      Kurt Hornik
    

    

      
      #
    

  


  

          

      
      
      
    

        
TTLAM> Kurt Hornik writes

            

      
      
      
    

        
TTLAM> Yes definitely. This line is in fact just a relic from a previous idea I
TTLAM> had.

            

      
      
      
    

        
TTLAM> Sounds reasonable. Ack.

            

      
      
      
    

        
Yes, of course.  But then we just made another backwards incompatible
change, and

R> c(ordered(1 : 3), ordered(4 : 6))
[1] 1 2 3 1 2 3

seems more like a bug to me :-)

            

      
      
      
    

        
Yes!

Best
-k

            

      
      
      
    

        
TTLAM> BR Thorn

TTLAM> ______________________________________________
TTLAM> R-devel at r-project.org mailing list
TTLAM> https://stat.ethz.ch/mailman/listinfo/r-devel