RobinHankin / disordR

https://robinhankin.github.io/disordR/
1 stars 0 forks source link

extract all elements #14

Closed RobinHankin closed 3 years ago

RobinHankin commented 3 years ago

Currently extraction by number is disallowed. But sometimes it makes sense:

> a <- disord(runif(5))
> a[1:5]
Error in .local(x, i, j = j, ..., drop) : 
  cannot use a regular index to extract, only a disord object
> 

Above, it would be reasonable to return a because all elements have been selected.

RobinHankin commented 3 years ago

Not at all sure about this. What should a[1:5] + a[5:1] return? Is this defined? Maybe a[1:5] should return a but with a different hash?

RobinHankin commented 3 years ago

Of course, a[c(1,1,2,3,4,5)] is clearly illegal even though all elements have been selected (which one should appear twice?)

RobinHankin commented 3 years ago

See also issue #15

RobinHankin commented 3 years ago

Unexpected behaviour of regular vectors:

> a <- 1:10
> a[c(T,T,T)]
 [1]  1  2  3  4  5  6  7  8  9 10
> 

This makes my first attempt unexpectedly good:

setMethod("[", signature("disord",i="index",j="missing",drop="ANY"),
          function(x,i,j,drop){
              jj <- seq_along(x)
              print(sort(jj[i]))
              print(jj)
              if(identical(sort(jj[i]),jj)){
                  return(x[])
              } else {
                  stop("cannot use a regular index to extract, only a disord object")
              }
          } )
RobinHankin commented 3 years ago

On reflection, it is much more consistent to have a blanket ban on extraction via a regular index, particularly because replacement methods would also have to be implemented (as in a <- rdis(9) ; a[1:9] <- a[1:9] + 6). Of course, to accomplish this one would use a <- a+6.

RobinHankin commented 3 years ago

reopening. Function zapsmall() fails on the disord objects:

> x <- 0.1^(0:9)
> x
 [1] 1e+00 1e-01 1e-02 1e-03 1e-04 1e-05 1e-06 1e-07 1e-08 1e-09
> zapsmall(x)
 [1] 1e+00 1e-01 1e-02 1e-03 1e-04 1e-05 1e-06 1e-07 0e+00 0e+00
> zapsmall(disord(x))
Error in .local(x, i, j = j, ..., drop) : 
  if using a regular index to extract, must extract all elements
> 

The reason it fails is that zapsmall() tries to extract just the non-NA elements of its argument x. Code snippet:

if (all(ina <- is.na(x))) 
        return(x)
    mx <- max(abs(x[!ina]))

failure on the last line which is effectively x[c(T,T,T,T)] where x is a disord of length 4. This is a good reason to let this idiom work, I think.

RobinHankin commented 3 years ago

the reasoning behind the "blanket ban" is flawed. There is nothing wrong with having an occasional non-error. Also there is something wrong with the comments in this issue, because b83342c implemented the extract-all-elements feature and then I said I wanted a blanket ban and then I wanted to implement extract-all-elements (because of zapsmall()) even though it was already implemented.