Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider doing missing value propagation before comparison check in iv() #36

Closed
DavisVaughan opened this issue Oct 9, 2022 · 1 comment · Fixed by #40
Closed

Consider doing missing value propagation before comparison check in iv() #36

DavisVaughan opened this issue Oct 9, 2022 · 1 comment · Fixed by #40

Comments

@DavisVaughan
Copy link
Owner

Also affects iv_diff()

We probably need to do this because vec_compare() can "early exit" if it can determine the comparison value early on. This can cause some confusing errors since we normally say that incomplete rows are going to be propagated as missing rows in the result, and I think that should take precedence over the comparison.

library(ivs)
library(vctrs)

# Can determine comparison early on
start <- data_frame(x = 1, y = NA)
start
#>   x  y
#> 1 1 NA

end <- data_frame(x = 0, y = 2)
end
#>   x y
#> 1 0 2

iv(start, end)
#> Error in `iv()`:
#> ! `start` must be less than `end`.
#> ℹ `start` is not less than `end` at locations: `c(1)`.

#> Backtrace:
#>     ▆
#>  1. └─ivs::iv(start, end)
#>  2.   └─rlang::abort(message) at ivs/R/iv.R:184:4

iv_diff(vec_c(start, end))
#> Error in `iv()` at ivs/R/diff.R:100:2:
#> ! `start` must be less than `end`.
#> ℹ `start` is not less than `end` at locations: `c(1)`.

#> Backtrace:
#>     ▆
#>  1. └─ivs::iv_diff(vec_c(start, end))
#>  2.   └─ivs::iv(start, end) at ivs/R/diff.R:100:2
#>  3.     └─rlang::abort(message) at ivs/R/iv.R:184:4

# Can't determine comparison because of `NA`
start <- data_frame(x = NA, y = 3)
start
#>    x y
#> 1 NA 3

end <- data_frame(x = 0, y = 2)
end
#>   x y
#> 1 0 2

iv(start, end)
#> <iv<data.frame<
#>   x: double
#>   y: double
#> >>[1]>
#> [1] [NA, NA)

iv_diff(vec_c(start, end))
#> <iv<data.frame<
#>   x: double
#>   y: double
#> >>[1]>
#> [1] [NA, NA)

Created on 2022-10-09 with reprex v2.0.2.9000

@DavisVaughan
Copy link
Owner Author

We want incompleteness to be handled first because if you switch these around to something like this:

start <- data_frame(x = 0, y = 2)
end <- data_frame(x = 1, y = NA)

then the comparison can also be made early, it looks like start < end, but it STILL doesn't matter in the end because the incompleteness check would kick in and result in a missing interval.

That means it should be clear that we want to apply this rule symmetrically, meaning that incompleteness should be done up front before any actual comparisons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant