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.

Sign up for GitHub

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

cbind: unique/duplicated colnamesare left asis/made unique; read10xVisium: keep barcodes as colData; spatialCoords/<-: withDimnames argument #128

Open
wants to merge 12 commits into
base: devel
Choose a base branch
from
Open

cbind: unique/duplicated colnamesare left asis/made unique; read10xVisium: keep barcodes as colData; spatialCoords/<-: withDimnames argument #128

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cc93497
assure cbind gives unique colnames & barcodes are kept as colData
CATALYST-project Nov 17, 2022
8d81990
Update R-cmd-check.yaml
drighelli Nov 17, 2022
3ddd2c3
+withDimnames arg in spatialCoords getter&setter
CATALYST-project Nov 21, 2022
d8aabf6
add doc for param 'withDimnames'
CATALYST-project Nov 21, 2022
ac4bd0f
Merge branch 'cbind_unique_colnms' into spatialCoords
CATALYST-project Nov 21, 2022
1b4fad4
version bump to v1.9.4
CATALYST-project Nov 22, 2022
fe40c8f
fix bug in unit tests (uninitialized variable)
CATALYST-project Nov 22, 2022
2831903
resolve merge conflicts
CATALYST-project Nov 22, 2022
8842c22
update NEWS file v1.9.4
CATALYST-project Nov 24, 2022
fc2e6af
fix generic/method defintiions
CATALYST-project Nov 24, 2022
253e5e4
add doc for ... param
CATALYST-project Nov 24, 2022
0db2ad7
Merge branch 'devel' into cbind_unique_colnms
drighelli Sep 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions .github/workflows/R-cmd-check.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@ jobs:
fail-fast: false
matrix:
config:
- { os: windows-latest, r: '4.2', bioc: 'devel'}
- { os: macOS-latest, r: '4.2', bioc: 'devel'}
- { os: ubuntu-latest, r: '4.2', bioc: 'devel', cont: "bioconductor/bioconductor_docker:devel", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
#- { os: windows-latest, r: '4.2', bioc: '3.15'}
#- { os: macOS-latest, r: '4.2', bioc: '3.15'}
#- { os: ubuntu-latest, r: '4.2', bioc: '3.15', cont: "bioconductor/bioconductor_docker:RELEASE_3_15", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
- { os: ubuntu-latest, bioc: 'devel'}
- { os: windows-latest, r: 'devel', bioc: 'devel'}
- { os: macOS-latest, r: 'devel', bioc: 'devel'}
#- { os: ubuntu-latest, r: '4.2', bioc: '3.16', cont: "bioconductor/bioconductor_docker:RELEASE_3_16", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
#- { os: windows-latest, r: '4.2', bioc: '3.16'}
#- { os: macOS-latest, r: '4.2', bioc: '3.16'}
#- { os: ubuntu-latest, r: '4.2', bioc: 'devel', cont: "bioconductor/bioconductor_docker:devel", rspm: "https://packagemanager.rstudio.com/cran/__linux__/focal/latest" }
#- { os: windows-latest, r: '4.2', bioc: 'devel'}
#- { os: macOS-latest, r: '4.2', bioc: 'devel'}

env:
R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
Expand All @@ -36,14 +39,14 @@ jobs:
- name: Check out repo
uses: actions/checkout@v2

- name: Set up R
uses: r-lib/actions/setup-r@master
- name: Set up R and install BiocManager
uses: grimbough/bioc-actions/setup-bioc@v1
if: matrix.config.image == null
with:
r-version: ${{ matrix.config.r }}
bioc-version: ${{ matrix.config.bioc }}

- name: Set up pandoc
uses: r-lib/actions/setup-pandoc@master
uses: r-lib/actions/setup-pandoc@v2
if: matrix.config.image == null

- name: Install windows dependencies
Expand Down
2 changes: 1 addition & 1 deletion DESCRIPTION
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ Suggests:
BiocStyle,
BumpyMatrix
VignetteBuilder: knitr
RoxygenNote: 7.2.1
RoxygenNote: 7.2.2
4 changes: 2 additions & 2 deletions R/AllGenerics.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ setGeneric("spatialDataNames", function(x) standardGeneric("spatialDataNames"))
setGeneric("spatialDataNames<-", function(x, value) standardGeneric("spatialDataNames<-"))

#' @export
setGeneric("spatialCoords", function(x, ...) standardGeneric("spatialCoords"))
setGeneric("spatialCoords", function(x, withDimnames=TRUE, ...) standardGeneric("spatialCoords"))

#' @export
setGeneric("spatialCoords<-", function(x, value) standardGeneric("spatialCoords<-"))
setGeneric("spatialCoords<-", function(x, value, withDimnames=TRUE) standardGeneric("spatialCoords<-"))

#' @export
setGeneric("spatialCoordsNames", function(x) standardGeneric("spatialCoordsNames"))
Expand Down
5 changes: 5 additions & 0 deletions R/SpatialExperiment-combine.R → R/SpatialExperiment-cbind.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ setMethod("cbind", "SpatialExperiment", function(..., deparse.level=1) {
out <- do.call(
callNextMethod,
c(args, list(deparse.level=1)))
if (any(duplicated(colnames(out)))) {
n <- vapply(args, ncol, integer(1))
n <- rep.int(seq_along(args), n)
colnames(out) <- paste(n, colnames(out), sep="_")
}

# merge 'imgData' from multiple samples
if (!is.null(imgData(args[[1]]))) {
Expand Down
69 changes: 6 additions & 63 deletions R/SpatialExperiment-methods.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@
#' identifier(s) for \code{scaleFactors}. Default = \code{TRUE} (all samples).
#' @param image_id Logical value or character vector specifying image
#' identifier(s) for \code{scaleFactors}. Default = \code{TRUE} (all images).
#' @param withDimnames Logical value indicating whether dimnames of the
#' \code{spatialExperiment} should be applied or checked against.
#' If \code{withDimnames=TRUE}, non-\code{NULL} \code{rownames(value)}
#' are checked against \code{colnames(x)}, and an error occurs if these
#' don't match. Else, discrepancies in rownames are ignored.
#' (see also \code{\link[SingleCellExperiment]{reducedDims}})
#'
#' @details
#' Additional details for each type of data attribute are provided below.
Expand Down Expand Up @@ -183,69 +189,6 @@ setReplaceMethod("spatialDataNames",
}
)

# spatialCoords ----------------------------------------------------------------

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData<-
#' @export
setMethod("spatialCoords",
"SpatialExperiment",
function(x) int_colData(x)$spatialCoords)

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData<-
#' @export
setReplaceMethod("spatialCoords",
c("SpatialExperiment", "matrix"),
function(x, value) {
stopifnot(
is.numeric(value),
nrow(value) == ncol(x))
int_colData(x)$spatialCoords <- value
return(x)
}
)

#' @rdname SpatialExperiment-methods
#' @export
setReplaceMethod("spatialCoords",
c("SpatialExperiment", "NULL"),
function(x, value) {
value <- matrix(numeric(), ncol(x), 0)
`spatialCoords<-`(x, value)
}
)

# spatialCoordsNames -----------------------------------------------------------

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData
#' @export
setMethod("spatialCoordsNames",
"SpatialExperiment",
function(x) colnames(int_colData(x)$spatialCoords))

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData<-
#' @export
setReplaceMethod("spatialCoordsNames",
c("SpatialExperiment", "character"),
function(x, value) {
colnames(int_colData(x)$spatialCoords) <- value
return(x)
}
)

#' @rdname SpatialExperiment-methods
#' @export
setReplaceMethod("spatialCoordsNames",
c("SpatialExperiment", "NULL"),
function(x, value) {
value <- character()
`spatialCoordsNames<-`(x, value)
}
)

# scaleFactors -----------------------------------------------------------------

#' @rdname SpatialExperiment-methods
Expand Down
8 changes: 3 additions & 5 deletions R/read10xVisium.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,11 @@ read10xVisium <- function(samples="",
cnms <- c(
"barcode", "in_tissue", "array_row", "array_col",
"pxl_row_in_fullres", "pxl_col_in_fullres")
df <- lapply(seq_along(x), function(i)
{
df <- read.csv(x[i],
header=!grepl("list", x[i]),
row.names=1, col.names=cnms)
df <- lapply(seq_along(x), function(i) {
df <- read.csv(x[i], header=!grepl("list", x[i]), col.names=cnms)
if (length(x) > 1) rownames(df) <- paste(i, rownames(df), sep="_")
if (!is.null(names(x))) cbind(sample_id=names(x)[i], df)
rownames(df) <- df$barcode
df
})
df <- do.call(rbind, df)
Expand Down
76 changes: 76 additions & 0 deletions R/spatialCoords.R
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@

# spatialCoords ----------------------------------------------------------------

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData<-
#' @export
setMethod("spatialCoords",
"SpatialExperiment",
function(x, withDimnames=TRUE) {
out <- int_colData(x)$spatialCoords
if (withDimnames)
rownames(out) <- colnames(x)
return(out)
})

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData<-
#' @export
setReplaceMethod("spatialCoords",
c("SpatialExperiment", "matrix"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a warning that I suppose is coming from the "matrix" instead of a "value"

Warning:     ‘spatialCoords<-’
    ‘\S4method{spatialCoords<-}{SpatialExperiment,NULL}’
    ‘\S4method{spatialCoords<-}{SpatialExperiment,matrix}’
  The argument of a replacement function which corresponds to the right
  hand side must be named ‘value’.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I saw. But I don’t understand because it’s good locally. I’m on it…

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed I don't get why, but have you tried something like: c("SpatialExperiment", "value") ?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's because the generic is defined as setGeneric("spatialCoords<-", function(x, value, withDimnames=TRUE) standardGeneric("spatialCoords<-"))

Copy link
Collaborator Author

@HelenaLC HelenaLC Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…the signature has to be classes, so “NULL” and “matrix” (not “value”) is correct. It’s saying the generic and methods don’t match. But I don’t see why not as they both have “value” in the function definition in the same order…

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I saw that, I'm looking over the internet, but still I'm not able to understand the motivation for this warning.

function(x, value, withDimnames=TRUE) {
stopifnot(
is.numeric(value),
nrow(value) == ncol(x))
new <- rownames(value)
if (!is.null(new) && withDimnames) {
if (!identical(new, colnames(x))) {
stop("Non-NULL 'rownames(value)' should be the",
" same as 'colnames(x)' for 'spatialCoords<-'.",
" Use 'withDimnames=FALSE' to force replacement.")
}
}
int_colData(x)$spatialCoords <- value
return(x)
}
)

#' @rdname SpatialExperiment-methods
#' @export
setReplaceMethod("spatialCoords",
c("SpatialExperiment", "NULL"),
function(x, value, withDimnames=TRUE) {
value <- matrix(numeric(), ncol(x), 0)
`spatialCoords<-`(x, value)
}
)

# spatialCoordsNames -----------------------------------------------------------

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData
#' @export
setMethod("spatialCoordsNames",
"SpatialExperiment",
function(x) colnames(int_colData(x)$spatialCoords))

#' @rdname SpatialExperiment-methods
#' @importFrom SingleCellExperiment int_colData<-
#' @export
setReplaceMethod("spatialCoordsNames",
c("SpatialExperiment", "character"),
function(x, value) {
colnames(int_colData(x)$spatialCoords) <- value
return(x)
}
)

#' @rdname SpatialExperiment-methods
#' @export
setReplaceMethod("spatialCoordsNames",
c("SpatialExperiment", "NULL"),
function(x, value) {
value <- character()
`spatialCoordsNames<-`(x, value)
}
)
38 changes: 23 additions & 15 deletions man/SpatialExperiment-methods.Rd
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 12 additions & 1 deletion tests/testthat/test_SpatialExperiment-cbind.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ test_that("duplicated sample_ids are made unique with a message", {
expect_true(nrow(new) == nrow(spe))
expect_true(ncol(new) == 2*ncol(spe))
expect_identical(rownames(new), rownames(spe))
expect_setequal(colnames(new), colnames(spe))
})

test_that("imgData are combined correctly", {
Expand All @@ -33,3 +32,15 @@ test_that("imgData are combined correctly", {
expect_identical(imgData(spe3)[one, ], imgData(spe1))
expect_identical(imgData(spe3)[two, ], imgData(spe2))
})

test_that("unique colnames are left asis,", {
tmp <- spe
colnames(tmp) <- paste0(colnames(tmp), "x")
out <- cbind(spe, tmp)
expect_false(any(duplicated(colnames(out))))
})

test_that("duplicated colnames are made unique", {
out <- cbind(spe, spe)
expect_false(any(duplicated(colnames(out))))
})
22 changes: 0 additions & 22 deletions tests/testthat/test_SpatialExperiment-methods.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,28 +45,6 @@ test_that("spatialDataNames()<-,NULL", {
expect_identical(new, character(0))
})

test_that("spatialCoordsNames()", {
expect_identical(
spatialCoordsNames(spe),
colnames(int_colData(spe)$spatialCoords))
})

test_that("spatialCoordsNames<-,character", {
old <- spatialCoordsNames(spe)
new <- sample(letters, length(old))
spatialCoordsNames(spe) <- new
expect_identical(spatialCoordsNames(spe), new)
expect_identical(spatialCoordsNames(spe),
colnames(int_colData(spe)$spatialCoords))
})

test_that("spatialCoordsNames<-,NULL", {
old <- spatialCoords(spe)
spatialCoordsNames(spe) <- NULL
expect_null(spatialCoordsNames(spe))
expect_equivalent(spatialCoords(spe), old)
})

test_that("scaleFactors()", {
sfs <- scaleFactors(spe, sample_id=TRUE, image_id=TRUE)
expect_is(sfs, "numeric")
Expand Down
Loading