From 9986d0c80cdaa655fc40bd98c8c43430e01b138d Mon Sep 17 00:00:00 2001
From: Artur-man <artur-man@hotmail.com>
Date: Fri, 23 May 2025 14:42:14 +0200
Subject: [PATCH 1/2] fix val/set/getTable bug for colData variables

---
 R/table_utils.R              | 4 +++-
 man/SpatialData.Rd           | 8 ++++----
 tests/testthat/test-tables.R | 8 ++++++--
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/R/table_utils.R b/R/table_utils.R
index bf5cd1c..591d535 100644
--- a/R/table_utils.R
+++ b/R/table_utils.R
@@ -125,7 +125,9 @@ setMethod("getTable", c("SpatialData", "character"), \(x, i, drop=TRUE) {
     # only keep observations belonging to 'i' (optional)
     if (drop) {
         rk <- meta(t)$region_key
-        t <- t[, int_colData(t)[[rk]] == i]
+        coldata <-
+          if(rk %in% names(cd <- int_colData(t))) cd[[rk]] else colData(t)[[rk]]
+        t <- t[, coldata == i]
     }
     return(t)
 })
diff --git a/man/SpatialData.Rd b/man/SpatialData.Rd
index 9521d85..e760cfd 100644
--- a/man/SpatialData.Rd
+++ b/man/SpatialData.Rd
@@ -50,8 +50,8 @@
 \alias{element,SpatialData,ANY,numeric-method}
 \alias{element,SpatialData,ANY,missing-method}
 \alias{element,SpatialData,ANY,ANY-method}
-\alias{[[<-,SpatialData,numeric,ANY,ANY-method}
-\alias{[[<-,SpatialData,character,ANY,ANY-method}
+\alias{[[<-,SpatialData,numeric,ANY-method}
+\alias{[[<-,SpatialData,character,ANY-method}
 \title{The `SpatialData` class}
 \usage{
 SpatialData(images, labels, points, shapes, tables)
@@ -88,9 +88,9 @@ SpatialData(images, labels, points, shapes, tables)
 
 \S4method{element}{SpatialData,ANY,ANY}(x, i, j)
 
-\S4method{[[}{SpatialData,numeric,ANY,ANY}(x, i) <- value
+\S4method{[[}{SpatialData,numeric,ANY}(x, i) <- value
 
-\S4method{[[}{SpatialData,character,ANY,ANY}(x, i) <- value
+\S4method{[[}{SpatialData,character,ANY}(x, i) <- value
 }
 \arguments{
 \item{images}{list of \code{\link{ImageArray}}s}
diff --git a/tests/testthat/test-tables.R b/tests/testthat/test-tables.R
index 3d8f112..cd83a23 100644
--- a/tests/testthat/test-tables.R
+++ b/tests/testthat/test-tables.R
@@ -48,7 +48,10 @@ test_that("getTable()", {
     # alter 'region' of a couple random observations
     s <- t
     . <- sample(ncol(s), 2)
-    int_colData(s)[[rk]][.] <- "."
+    # int_colData(s)[[rk]][.] <- "."
+    tmp <- as.character(colData(s)[[rk]])
+    tmp[.] <- "."
+    colData(s)[[rk]] <- tmp
     SpatialData::table(x) <- s
     # these should be gone when 'drop=TRUE'
     t1 <- getTable(x, i, drop=FALSE)
@@ -121,7 +124,8 @@ test_that("valTable()", {
     expect_error(valTable(x, i, sample(rownames(t), 2)))
     expect_error(valTable(x, i, sample(names(colData(t)), 2)))
     # 'colData'
-    df <- DataFrame(a=sample(letters, n), b=runif(n))
+    df <- DataFrame(a=sample(letters, n), b=runif(n),
+                    region = valTable(x, i, j <- "region"))
     s <- t; colData(s) <- df; y <- x; SpatialData::table(y) <- s
     expect_identical(valTable(y, i, j <- "a"), s[[j]])
     expect_identical(valTable(y, i, j <- "b"), s[[j]])

From a0ddc8b082c8cf8429ed5bc00fd912052bb84a89 Mon Sep 17 00:00:00 2001
From: Artur-man <artur-man@hotmail.com>
Date: Fri, 23 May 2025 15:07:32 +0200
Subject: [PATCH 2/2] add notes

---
 R/table_utils.R              | 2 ++
 tests/testthat/test-tables.R | 1 +
 2 files changed, 3 insertions(+)

diff --git a/R/table_utils.R b/R/table_utils.R
index 591d535..b875a0e 100644
--- a/R/table_utils.R
+++ b/R/table_utils.R
@@ -125,6 +125,8 @@ setMethod("getTable", c("SpatialData", "character"), \(x, i, drop=TRUE) {
     # only keep observations belonging to 'i' (optional)
     if (drop) {
         rk <- meta(t)$region_key
+        # TODO: check the replacement below, search colData as well?
+        # t <- t[, int_colData(t)[[rk]] == i]
         coldata <-
           if(rk %in% names(cd <- int_colData(t))) cd[[rk]] else colData(t)[[rk]]
         t <- t[, coldata == i]
diff --git a/tests/testthat/test-tables.R b/tests/testthat/test-tables.R
index cd83a23..74e5f6b 100644
--- a/tests/testthat/test-tables.R
+++ b/tests/testthat/test-tables.R
@@ -48,6 +48,7 @@ test_that("getTable()", {
     # alter 'region' of a couple random observations
     s <- t
     . <- sample(ncol(s), 2)
+    # TODO: check the replacement of this test below
     # int_colData(s)[[rk]][.] <- "."
     tmp <- as.character(colData(s)[[rk]])
     tmp[.] <- "."