R devel - Jan 2020 - [patch] add sanity checks to quantile()

The attached patch adds some sanity checks to the "type" argument of
quantile(). Output from the following commands show the change of
behavior with the current patch:

  vec <- 1:10
  quantile(vec, type = c(1, 2))
  quantile(vec, type = 10)
  quantile(vec, type = "aaa")
  quantile(vec, type = NA_real_)
  quantile(vec, type = 4.3)
  quantile(vec, type = -1)

Current behavior (i.e., without the patch):

  > vec <- 1:10
  > quantile(vec, type = c(1, 2))
  Error in switch(type, (nppm > j), ((nppm > j) + 1)/2, (nppm != j) |
((j%%2L) ==  :
    EXPR must be a length 1 vector
  In addition: Warning messages:
  1: In if (type == 7) { :
    the condition has length > 1 and only the first element will be used
  2: In if (type <= 3) { :
    the condition has length > 1 and only the first element will be used
  3: In if (type == 3) n * probs - 0.5 else n * probs :
    the condition has length > 1 and only the first element will be used
  > quantile(vec, type = 10)
  Error in quantile.default(vec, type = 10) : object 'a' not found
  > quantile(vec, type = "aaa")
  Error in type - 3 : non-numeric argument to binary operator
  > quantile(vec, type = NA_real_)
  Error in if (type == 7) { : missing value where TRUE/FALSE needed
  > quantile(vec, type = 4.3)
    0%  25%  50%  75% 100% 
   1.0  2.5  5.0  7.5 10.0 
  > quantile(vec, type = -1)
    0%  25%  50%  75% 100% 
     1    2    5    7   10 


Behavior with the patch:

  > vec <- 1:10
  > quantile(vec, type = c(1, 2))
  Error in quantile.default(vec, type = c(1, 2)) : 
    'type' must be of length 1
  > quantile(vec, type = 10)
  Error in quantile.default(vec, type = 10) : 
    'type' must be an integer between 1 and 9
  > quantile(vec, type = "aaa")
  Error in quantile.default(vec, type = "aaa") : 
    'type' must be an integer between 1 and 9
  > quantile(vec, type = NA_real_)
  Error in quantile.default(vec, type = NA_real_) : 
    'type' must be an integer between 1 and 9
  > quantile(vec, type = 4.3)
  Error in quantile.default(vec, type = 4.3) : 
    'type' must be an integer between 1 and 9
  > quantile(vec, type = -1)
  Error in quantile.default(vec, type = -1) : 
    'type' must be an integer between 1 and 9


Note that with the patch, quantile() gives an error in some cases where
the current code does not. Specifically, the following two calls to
quantile() do not give an error without the patch:

  quantile(vec, type = 4.3)
  quantile(vec, type = -1)

Thus, this patch could cause current code to give an error. If it is
desired, I could change the patch such that it only gives an error when
current R gives an error (i.e., the only benefit of the patch would be
better error messages), or I can change the patch to give a warning in
these cases.

Scott


-- 
Scott Kostyshak
Assistant Professor of Economics
University of Florida
https://people.clas.ufl.edu/skostyshak/


-------------- next part --------------
A non-text attachment was scrubbed...
Name: quantile.diff
Type: text/x-diff
Size: 623 bytes
Desc: not available
URL:
<https://stat.ethz.ch/pipermail/r-devel/attachments/20190531/f5cdbe7c/attachment.bin>

On Fri, May 31, 2019 at 01:28:55AM -0400, Scott Kostyshak
wrote:
> The attached patch adds some sanity checks to the "type" argument
of
> quantile(). Output from the following commands show the change of
> behavior with the current patch:
> 
>   vec <- 1:10
>   quantile(vec, type = c(1, 2))
>   quantile(vec, type = 10)
>   quantile(vec, type = "aaa")
>   quantile(vec, type = NA_real_)
>   quantile(vec, type = 4.3)
>   quantile(vec, type = -1)
> 
> Current behavior (i.e., without the patch):
> 
>   > vec <- 1:10
>   > quantile(vec, type = c(1, 2))
>   Error in switch(type, (nppm > j), ((nppm > j) + 1)/2, (nppm != j) |
((j%%2L) ==  :
>     EXPR must be a length 1 vector
>   In addition: Warning messages:
>   1: In if (type == 7) { :
>     the condition has length > 1 and only the first element will be used
>   2: In if (type <= 3) { :
>     the condition has length > 1 and only the first element will be used
>   3: In if (type == 3) n * probs - 0.5 else n * probs :
>     the condition has length > 1 and only the first element will be used
>   > quantile(vec, type = 10)
>   Error in quantile.default(vec, type = 10) : object 'a' not found
>   > quantile(vec, type = "aaa")
>   Error in type - 3 : non-numeric argument to binary operator
>   > quantile(vec, type = NA_real_)
>   Error in if (type == 7) { : missing value where TRUE/FALSE needed
>   > quantile(vec, type = 4.3)
>     0%  25%  50%  75% 100% 
>    1.0  2.5  5.0  7.5 10.0 
>   > quantile(vec, type = -1)
>     0%  25%  50%  75% 100% 
>      1    2    5    7   10 
> 
> 
> Behavior with the patch:
> 
>   > vec <- 1:10
>   > quantile(vec, type = c(1, 2))
>   Error in quantile.default(vec, type = c(1, 2)) : 
>     'type' must be of length 1
>   > quantile(vec, type = 10)
>   Error in quantile.default(vec, type = 10) : 
>     'type' must be an integer between 1 and 9
>   > quantile(vec, type = "aaa")
>   Error in quantile.default(vec, type = "aaa") : 
>     'type' must be an integer between 1 and 9
>   > quantile(vec, type = NA_real_)
>   Error in quantile.default(vec, type = NA_real_) : 
>     'type' must be an integer between 1 and 9
>   > quantile(vec, type = 4.3)
>   Error in quantile.default(vec, type = 4.3) : 
>     'type' must be an integer between 1 and 9
>   > quantile(vec, type = -1)
>   Error in quantile.default(vec, type = -1) : 
>     'type' must be an integer between 1 and 9
> 
> 
> Note that with the patch, quantile() gives an error in some cases where
> the current code does not. Specifically, the following two calls to
> quantile() do not give an error without the patch:
> 
>   quantile(vec, type = 4.3)
>   quantile(vec, type = -1)
> 
> Thus, this patch could cause current code to give an error. If it is
> desired, I could change the patch such that it only gives an error when
> current R gives an error (i.e., the only benefit of the patch would be
> better error messages), or I can change the patch to give a warning in
> these cases.
> 
> Scott
> 
> 
> -- 
> Scott Kostyshak
> Assistant Professor of Economics
> University of Florida
> https://people.clas.ufl.edu/skostyshak/
> 
Bump. For this type of patch proposal, is it better to use the
bug tracker?

Thanks,

Scott


-- 
Scott Kostyshak
Assistant Professor of Economics
University of Florida
https://people.clas.ufl.edu/skostyshak/

On 04/01/2020 4:35 p.m., Scott Kostyshak wrote:
> On Fri, May 31, 2019 at 01:28:55AM -0400, Scott Kostyshak wrote:
>> The attached patch adds some sanity checks to the "type"
argument of
...
> Bump. For this type of patch proposal, is it better to use the
> bug tracker?
For almost any patch proposal it is.  Certainly if you don't get action 
(or at least discussion) within a few days, any other proposal will be 
forgotten.

Duncan Murdoch