Skip to content

Commit

Permalink
determine compose_trans domains by pushing domain forward/back throug…
Browse files Browse the repository at this point in the history
…h transforms (#408)
  • Loading branch information
mjskay authored Nov 6, 2023
1 parent 33d0e97 commit eba8e99
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* Add an inverse (area) hyperbolic sine transformation `asinh_trans()`, which
provides a logarithm-like transformation of a space, but which accommodates
negative values (#297)
* Correct the domain calculation for `compose_trans()` (@mjskay, #408).
* Transformation objects can optionally include the derivatives of the transform
and the inverse transform (@mjskay, #322).

Expand Down
20 changes: 16 additions & 4 deletions R/trans-compose.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,22 @@ compose_trans <- function(...) {
cli::cli_abort("{.fun compose_trans} must include at least 1 transformer to compose")
}

# Resolve domains
suppressWarnings(
domain <- compose_fwd(trans_list[[1]]$domain, trans_list[-1])
)
# Resolve domains. First push the domain of the first transformation all the
# way forward through the sequence of transformations, intersecting it with
# all domains along the way, to get the range. Then push the range back
# through the inverses to get the domain.
range <- trans_list[[1]]$transform(trans_list[[1]]$domain)
for (trans in trans_list[-1]) {
lower <- max(min(trans$domain), min(range))
upper <- min(max(trans$domain), max(range))
if (isTRUE(lower <= upper)) {
range <- trans$transform(c(lower, upper))
} else {
range <- c(NA_real_, NA_real_)
break
}
}
domain <- compose_rev(range, trans_list)
if (any(is.na(domain))) {
cli::cli_abort("Sequence of transformations yields invalid domain")
}
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/_snaps/trans-compose.md
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
Error in `compose_trans()`:
! `compose_trans()` must include at least 1 transformer to compose
Code
compose_trans("reverse", "log10")
compose_trans("sqrt", "reverse", "log10")
Condition
Error in `compose_trans()`:
! Sequence of transformations yields invalid domain
Expand Down
11 changes: 10 additions & 1 deletion tests/testthat/test-trans-compose.R
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ test_that("uses breaks from first transformer", {
test_that("produces informative errors", {
expect_snapshot(error = TRUE, {
compose_trans()
compose_trans("reverse", "log10")
compose_trans("sqrt", "reverse", "log10")
})
})

test_that("produces correct domains", {
expect_equal(compose_trans("sqrt", "reverse")$domain, c(0, Inf))
expect_equal(compose_trans("sqrt", "log")$domain, c(0, Inf))
expect_equal(compose_trans("log", "log")$domain, c(1, Inf))
expect_equal(compose_trans("reverse", "log")$domain, c(-Inf, 0))
expect_equal(compose_trans("reverse", "logit", "log")$domain, c(-1, -0.5))
expect_error(compose_trans("sqrt", "reverse", "log")$domain, "invalid domain")
})

0 comments on commit eba8e99

Please sign in to comment.