[patch] add sanity checks to quantile()

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] add sanity checks to quantile()

Scott Kostyshak-2
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/


______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

quantile.diff (640 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch] add sanity checks to quantile()

Scott Kostyshak-2
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/

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] add sanity checks to quantile()

Duncan Murdoch-2
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

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] add sanity checks to quantile()

Scott Kostyshak-2
On Sat, Jan 04, 2020 at 06:32:15PM -0500, Duncan Murdoch wrote:

>
> 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

That makes sense. Thanks for the quick reply and advice. Here is the
ticket:

  https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17683

Scott


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

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel