psolymos / intrval

Relational Operators for Intervals
41 stars 4 forks source link

show warning if the endpoint is NA #7

Closed mgacc0 closed 7 years ago

mgacc0 commented 7 years ago

It's very prudent to show a warning before dropping NA values:

.intrval(2, c(1, NA), "[]")
# [1] FALSE
# Warning message:
# In .get_intrval(interval) : NA values where found and coerced
psolymos commented 7 years ago

I was thinking about having na.rm=TRUE in pmin/pmax but decided not to, that is why I left na.rm=FALSE there as an explicit reminder. Here is my reasoning:

When is.na(x), there is not much one can do, and the results should be NA. But when the interval specification contains an NA I think the same should happen. In the case of c(1, NA) the na.rm=TRUE reduces the interval to a degenerate interval of a single value when closed [1,1], or nothing when open (1,1). Also there is the issue of soring: it cannot be assessed of the non NA value is the lower or the upper endpoint. This, I think is undesirable and should return an NA. When both endpoints are NAs, there is no issue.

When checking the code, I realized that I left in a na.rm=TRUE (here). That should be.

However, throwing a more specific warning is helpful.

mgacc0 commented 7 years ago

👍