A patch for do_sample: check replace arg

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

A patch for do_sample: check replace arg

Seth Falcon-2
A colleague sent me the following:

    If you specify probabilities in the 'sample' function and forget
    to type 'prob=...', then you get nonsense. E.g.
   
        sample(1:10,1,c(0,0,0,0,1,0,0,0,0,0))

    does not filter '5', while
   
        sample(1:10,1,prob=c(0,0,0,0,1,0,0,0,0,0))

    does it correctly.  I wish this would return an error because the
    'replace' argument should only take logical args. Anyway, it is
    easy to make this mistake and having it produce an error would be
    nice.

Assuming there is not a use-case for specifying a logical vector for
the 'replace' argument, I like the idea of raising an error if replace
is not length one.  The following patch provides an implementation.

+ seth


Diff is against svn Revision: 37141
--- a/src/main/random.c Sat Jan 21 10:54:11 2006 -0800
+++ b/src/main/random.c Sat Jan 21 11:17:20 2006 -0800
@@ -453,15 +453,18 @@
 /* with/without replacement according to r. */
 SEXP attribute_hidden do_sample(SEXP call, SEXP op, SEXP args, SEXP rho)
 {
-    SEXP x, y, prob;
+    SEXP x, y, prob, sreplace;
     int k, n, replace;
     double *p;
 
     checkArity(op, args);
     n = asInteger(CAR(args)); args = CDR(args);
     k = asInteger(CAR(args)); args = CDR(args);
-    replace = asLogical(CAR(args)); args = CDR(args);
+    sreplace = CAR(args); args = CDR(args);
     prob = CAR(args);
+    if (length(sreplace) != 1)
+        errorcall(call, _("invalid '%s' argument"), "replace");
+    replace = asLogical(sreplace);
     if (replace == NA_LOGICAL)
        errorcall(call, _("invalid '%s' argument"), "replace");
     if (n == NA_INTEGER || n < 1)

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

Re: A patch for do_sample: check replace arg

Robert Gentleman
should be there now

Seth Falcon wrote:

> A colleague sent me the following:
>
>     If you specify probabilities in the 'sample' function and forget
>     to type 'prob=...', then you get nonsense. E.g.
>    
>         sample(1:10,1,c(0,0,0,0,1,0,0,0,0,0))
>
>     does not filter '5', while
>    
>         sample(1:10,1,prob=c(0,0,0,0,1,0,0,0,0,0))
>
>     does it correctly.  I wish this would return an error because the
>     'replace' argument should only take logical args. Anyway, it is
>     easy to make this mistake and having it produce an error would be
>     nice.
>
> Assuming there is not a use-case for specifying a logical vector for
> the 'replace' argument, I like the idea of raising an error if replace
> is not length one.  The following patch provides an implementation.
>
> + seth
>
>
> Diff is against svn Revision: 37141
> --- a/src/main/random.c Sat Jan 21 10:54:11 2006 -0800
> +++ b/src/main/random.c Sat Jan 21 11:17:20 2006 -0800
> @@ -453,15 +453,18 @@
>  /* with/without replacement according to r. */
>  SEXP attribute_hidden do_sample(SEXP call, SEXP op, SEXP args, SEXP rho)
>  {
> -    SEXP x, y, prob;
> +    SEXP x, y, prob, sreplace;
>      int k, n, replace;
>      double *p;
>  
>      checkArity(op, args);
>      n = asInteger(CAR(args)); args = CDR(args);
>      k = asInteger(CAR(args)); args = CDR(args);
> -    replace = asLogical(CAR(args)); args = CDR(args);
> +    sreplace = CAR(args); args = CDR(args);
>      prob = CAR(args);
> +    if (length(sreplace) != 1)
> +        errorcall(call, _("invalid '%s' argument"), "replace");
> +    replace = asLogical(sreplace);
>      if (replace == NA_LOGICAL)
>         errorcall(call, _("invalid '%s' argument"), "replace");
>      if (n == NA_INTEGER || n < 1)
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

--
Robert Gentleman, PhD
Program in Computational Biology
Division of Public Health Sciences
Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N, M2-B876
PO Box 19024
Seattle, Washington 98109-1024
206-667-7700
[hidden email]

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