image/label transformations by HelenaLC · Pull Request #146 · HelenaLC/spatialdataR

HelenaLC · 2026-04-06T11:49:03Z

No description provided.

Copilot

Pull request overview

This PR adds support for applying translation() to sdArray objects (covering both ImageArray and LabelArray) and introduces test coverage for image/label translations.

Changes:

Implement translation,sdArray,numeric-method to pad/shift array-backed SpatialData elements.
Add testthat tests validating translation behavior for images, labels, and multiscale labels.
Update roxygen-generated docs and NAMESPACE imports to support the new method.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`R/trans.R`	Implements `translation()` for `sdArray` (image/label arrays), including multiscale handling.
`tests/testthat/test-trans.R`	Adds tests for `translation()` on images, labels, and multiscale labels.
`NAMESPACE`	Imports `DelayedArray::cbind` / `DelayedArray::rbind` to support the implementation.
`man/trans.Rd`	Documents the new S4 method alias/usage for `translation,sdArray,numeric`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T11:55:48Z

+importFrom(DelayedArray,cbind)
+importFrom(DelayedArray,rbind)


Importing cbind/rbind from DelayedArray into the package namespace can mask base cbind/rbind and affect other code paths in this package that call cbind() (e.g., data.frame/DFrame binding). Consider removing these importFrom() entries and instead calling DelayedArray::cbind() / DelayedArray::rbind() (or DelayedArray::bindCOLS/bindROWS) explicitly at the call sites that need DelayedArray support.

Suggested change

importFrom(DelayedArray,cbind)

importFrom(DelayedArray,rbind)

Copilot · 2026-04-06T11:55:49Z

+        if (length(dim(y)) == 2) {
+            n <- NULL
+        } else {
+            t <- t[-1]
+            n <- dim(x)[1] 
+            y <- aperm(y, c(2,3,1))
+        }


For 3D arrays you drop the first component of t (t <- t[-1]) and never use it. If a caller passes a non-zero translation for the first dimension, it will be silently ignored (and dim(y) will not match the requested translation). Either enforce t[1] == 0 when length(dim(y)) > 2, or implement translation along that dimension as well.

Copilot · 2026-04-06T11:55:49Z

+            z <- DelayedArray(array(0, d))
+            y <- if (t[2] > 0) cbind(z, y) else cbind(y, z)
+        }
+        if (t[1] != 0) {
+            d <- c(abs(t[1]), ncol(y), n)
+            z <- DelayedArray(array(0, d))


DelayedArray(array(0, d)) allocates a dense in-memory array for the padding block. For large images/labels and/or large translations this can cause significant memory use and slowdowns. Prefer a lazy constant/zero array (e.g., S4Arrays::ConstantArray(0, dim=d) / an equivalent DelayedArray/S4Arrays constructor) so the padding remains delayed.

Suggested change

z <- DelayedArray(array(0, d))

y <- if (t[2] > 0) cbind(z, y) else cbind(y, z)

}

if (t[1] != 0) {

d <- c(abs(t[1]), ncol(y), n)

z <- DelayedArray(array(0, d))

z <- S4Arrays::ConstantArray(0, dim=d)

y <- if (t[2] > 0) cbind(z, y) else cbind(y, z)

}

if (t[1] != 0) {

d <- c(abs(t[1]), ncol(y), n)

z <- S4Arrays::ConstantArray(0, dim=d)

Copilot · 2026-04-06T11:55:49Z

+    if (length(ys) == 1) {
+        ts <- list(t)
+    } else {
+        ds <- vapply(ys, ncol, integer(1))
+        sf <- c(1, ds[-1]/ds[-length(ds)])
+        ts <- lapply(cumprod(sf), `*`, t)


When x is multiscale, ts is computed by multiplying t by scale factors derived from ncol(). This can yield non-integer translations due to non-integer scale ratios and/or floating-point rounding (e.g., 0.5 * t becoming 31.999999), which will later be used as array dimensions via abs(t[1/2]) and can error. Consider validating/rounding each scaled translation (and failing with a clear error if it’s not exactly integer) before using it to construct d.

Suggested change

if (length(ys) == 1) {

ts <- list(t)

} else {

ds <- vapply(ys, ncol, integer(1))

sf <- c(1, ds[-1]/ds[-length(ds)])

ts <- lapply(cumprod(sf), `*`, t)

validate_translation <- \(tt, scale) {

rr <- round(tt)

tol <- sqrt(.Machine$double.eps)

if (!all(is.finite(tt)) || any(abs(tt - rr) > tol)) {

stop(

sprintf(

"translation becomes non-integer at multiscale level %d after scaling; got %s",

scale, paste(tt, collapse=", ")),

call.=FALSE)

}

as.integer(rr)

}

if (length(ys) == 1) {

ts <- list(as.integer(round(t)))

} else {

ds <- vapply(ys, ncol, integer(1))

sf <- c(1, ds[-1]/ds[-length(ds)])

ts <- lapply(seq_along(sf), \(k) {

validate_translation(cumprod(sf)[k] * t, k)

})

Copilot · 2026-04-06T11:55:49Z

+    # row
+    t <- c(0,n <- sample(77, 1),0)
+    y <- translation(x, t)
+    expect_equal(dim(y), dim(x)+t)
+    expect_is(data(y), "DelayedArray")
+    expect_true(sum(data(y)[,seq_len(n),]) == 0)
+    # col
+    t <- c(0,0,n <- sample(77, 1))
+    y <- translation(x, t)
+    expect_equal(dim(y), dim(x)+t)
+    expect_is(data(y), "DelayedArray")
+    expect_true(sum(data(y)[,,seq_len(n)]) == 0)


These tests use sample() to pick translation sizes. Even with a small range, randomness can make failures harder to reproduce and can introduce variability in runtime. Consider using fixed translation values (or setting a local seed inside each test_that) so the tests are fully deterministic.

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T12:48:55Z

+        length(t) == length(dim(x)), 
+        t == round(t), all(is.finite(t)))


stopifnot(t == round(t)) will always fail when t has length > 1 because stopifnot() expects each argument to be a single TRUE; a logical vector is not isTRUE(). This makes translation(sdArray, numeric) unusable for the intended 2D/3D cases. Use a scalar check like all(t == round(t)) (and keep the finiteness check scalar as well).

Suggested change

length(t) == length(dim(x)),

t == round(t), all(is.finite(t)))

length(t) == length(dim(x)),

all(t == round(t)),

all(is.finite(t)))

Copilot · 2026-04-06T12:48:55Z

+    replicate(10, \() {
+        n <- f(); m <- f()
+        y <- translation(x, c(n,m))
+        expect_identical(x$x, y$x+n)
+        expect_identical(x$y, y$y+m)
+        expect_identical(x[,i], y[i])


The replicate() call is not executing the expectations: replicate(10, \() { ... }) replicates the function object rather than running the body. Additionally, inside the body the translation assertions are currently reversed (expect_identical(x$x, y$x+n) should compare y$x to x$x + n) and y[i] subsets rows rather than the intended columns (y[, i]). As written this test will either do nothing (because the expectations never run) or fail once corrected to execute.

Suggested change

replicate(10, \() {

n <- f(); m <- f()

y <- translation(x, c(n,m))

expect_identical(x$x, y$x+n)

expect_identical(x$y, y$y+m)

expect_identical(x[,i], y[i])

replicate(10, {

n <- f(); m <- f()

y <- translation(x, c(n,m))

expect_identical(y$x, x$x+n)

expect_identical(y$y, x$y+m)

expect_identical(x[,i], y[,i])

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T13:28:56Z

+        mutate(a=x*R[1,1], b=y*R[1,2]) |>
+        mutate(c=x*R[2,1], d=y*R[2,2]) |>


.R() now returns the transpose of the usual CCW rotation matrix (suitable for right-multiplying row vectors, as used in ShapeFrame rotation via xy %*% .R(...)). However, PointFrame rotation computes x' = x*R[1,1] + y*R[1,2] / y' = x*R[2,1] + y*R[2,2] (left-multiplying column vectors), which makes the rotation direction opposite to ShapeFrame and the “counter-clockwise” comment. Please make the convention consistent (e.g., transpose R in the PointFrame method or adjust .R()/call sites) so the same angle rotates points in the same direction across element types.

Suggested change

mutate(a=x*R[1,1], b=y*R[1,2]) |>

mutate(c=x*R[2,1], d=y*R[2,2]) |>

mutate(a=x*R[1,1], b=y*R[2,1]) |>

mutate(c=x*R[1,2], d=y*R[2,2]) |>

HelenaLC added 2 commits April 6, 2026 13:48

get started on image/label transformations

f90c91d

update trans docs

8e146c7

HelenaLC requested a review from Copilot April 6, 2026 11:49

Copilot started reviewing on behalf of HelenaLC April 6, 2026 11:50 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

HelenaLC added 2 commits April 6, 2026 14:28

update SpatialData.data retrieval

c13dbab

+translation,PointFrame tests

d77d514

HelenaLC requested a review from Copilot April 6, 2026 12:43

Copilot started reviewing on behalf of HelenaLC April 6, 2026 12:43 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

HelenaLC added 2 commits April 6, 2026 14:56

+rotate,PointFrame tests

b35dd2c

unskip tests

34ffa00

HelenaLC requested a review from Copilot April 6, 2026 13:23

Copilot AI reviewed Apr 6, 2026

View reviewed changes

HelenaLC added 3 commits April 6, 2026 15:44

+scale,pointFrame tests

176f9fa

fix doc typo

311d400

update docs

8e8d1f9

HelenaLC merged commit a2cf633 into main Apr 6, 2026
3 checks passed

HelenaLC deleted the trans branch April 24, 2026 13:48

Artur-man mentioned this pull request May 15, 2026

transformations #89

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

image/label transformations#146

image/label transformations#146
HelenaLC merged 9 commits into
mainfrom
trans

HelenaLC commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		importFrom(DelayedArray,cbind)
		importFrom(DelayedArray,rbind)

-    if (length(ys) == 1) {
-        ts <- list(t)
-    } else {
-        ds <- vapply(ys, ncol, integer(1))
-        sf <- c(1, ds[-1]/ds[-length(ds)])
-        ts <- lapply(cumprod(sf), `*`, t)
+    validate_translation <- \(tt, scale) {
+        rr <- round(tt)
+        tol <- sqrt(.Machine$double.eps)
+        if (!all(is.finite(tt)) || any(abs(tt - rr) > tol)) {
+            stop(
+                sprintf(
+                    "translation becomes non-integer at multiscale level %d after scaling; got %s",
+                    scale, paste(tt, collapse=", ")),
+                call.=FALSE)
+        }
+        as.integer(rr)
+    }
+    if (length(ys) == 1) {
+        ts <- list(as.integer(round(t)))
+    } else {
+        ds <- vapply(ys, ncol, integer(1))
+        sf <- c(1, ds[-1]/ds[-length(ds)])
+        ts <- lapply(seq_along(sf), \(k) {
+            validate_translation(cumprod(sf)[k] * t, k)
+        })

		length(t) == length(dim(x)),
		t == round(t), all(is.finite(t)))

		mutate(a=xR[1,1], b=yR[1,2]) \|>
		mutate(c=xR[2,1], d=yR[2,2]) \|>

Conversation

HelenaLC commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants