-
Notifications
You must be signed in to change notification settings - Fork 10
per-layer validity #147
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
per-layer validity #147
Changes from all commits
bf0f42a
767fd75
98108e8
a573807
3f2058d
31c8189
79af773
da875c2
84a431d
9e785cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,64 +17,85 @@ | |
| return(msg) | ||
| } | ||
|
|
||
| .validatePointFrame <- function(object) { | ||
| msg <- NULL | ||
| # Checks if the points have the x,y coordinates, as they are hard-coded | ||
| # in the plot functions | ||
| if (length(points(object))) { # there are some cases where the points are empty | ||
| if (!is.null(data(point(object)))) { | ||
| np <- length(points(object)) | ||
| for (i in seq_len(np)) { | ||
| dfi <- data(point(object, i)) | ||
| if (!all(c("x", "y") %in% names(dfi))) { | ||
| msg <- c(msg, paste0("'x' and 'y' missing in data point ", i)) | ||
| } | ||
| } | ||
| } | ||
| .validateImageArray <- \(object) { | ||
| msg <- c() | ||
| res <- length(object) | ||
| for (k in seq_len(res)) { | ||
| x <- data(object, k) | ||
| if (length(dim(x)) != 3) msg <- c(msg, paste( | ||
| "'ImageArray' resolution", k, "is not 3D")) | ||
| if (!type(x) %in% c("double", "integer")) msg <- c(msg, paste( | ||
| "'ImageArray' resolution", k, "is not of type double or integer")) | ||
| } | ||
| return(msg) | ||
| } | ||
| #' @importFrom S4Vectors setValidity2 | ||
| setValidity2("ImageArray", .validateImageArray) | ||
|
|
||
| .validateImageArray <- function(object) { | ||
| #' @importFrom ZarrArray type | ||
| .validateLabelArray <- \(object) { | ||
| msg <- c() | ||
| if (ni <- length(images(object))) { | ||
| for (i in seq_len(ni)) { | ||
| ai <- as.array(aperm(data(image(object,1))/255, perm=c(3,2,1))) | ||
| for (j in seq_len(dim(ai)[3])) { | ||
| if (!all(vapply(ai[,,j], is.numeric, logical(1)))) { | ||
| msg <- c(msg, paste0("Image ", i, " channel ", j, " not numeric")) | ||
| } | ||
| } | ||
| } | ||
| res <- length(object) | ||
| for (k in seq_len(res)) { | ||
| x <- data(object, k) | ||
| if (length(dim(x)) != 2) msg <- c(msg, paste( | ||
| "'LabelArray' resolution", k, "is not 2D")) | ||
| if (type(x) != "integer") msg <- c(msg, paste( | ||
| "'LabelArray' resolution", k, "is not of type integer")) | ||
| } | ||
| return(msg) | ||
| } | ||
| #' @importFrom S4Vectors setValidity2 | ||
| setValidity2("LabelArray", .validateLabelArray) | ||
|
|
||
| .validatePointFrame <- \(object) { | ||
| msg <- c() | ||
| if (!length(object)) return(msg) | ||
| if (!"x" %in% names(object)) msg <- c(msg, "'PointFrame' missing 'x'.") | ||
| if (!"y" %in% names(object)) msg <- c(msg, "'PointFrame' missing 'y'.") | ||
| return(msg) | ||
| } | ||
| #' @importFrom S4Vectors setValidity2 | ||
| setValidity2("PointFrame", .validatePointFrame) | ||
|
|
||
| .validateShapeFrame <- \(object) { | ||
| msg <- c() | ||
| if (!nrow(object)) return(msg) | ||
| if (!"geometry" %in% names(object)) msg <- c(msg, "'ShapeFrame' missing 'geometry'.") | ||
| return(msg) | ||
| } | ||
| #' @importFrom S4Vectors setValidity2 | ||
| setValidity2("ShapeFrame", .validateShapeFrame) | ||
|
|
||
| #' @importFrom methods is | ||
| .validateSpatialData <- \(x) { | ||
| msg <- c() | ||
| typ <- c( | ||
| images="ImageArray", | ||
| labels="LabelArray", | ||
| points="PointFrame", | ||
| shapes="ShapeFrame", | ||
| tables="SingleCellExperiment") | ||
| msg <- NULL | ||
| for (. in names(typ)) if (length(x[[.]])) | ||
| if (!all(vapply(x[[.]], \(y) is(y, typ[.]), logical(1)))) | ||
| msg <- c(msg, sprintf("'%s' should be a list of '%s'", ., typ[.])) | ||
| msg <- c(msg, .validatePointFrame(x)) | ||
| # TODO: validate .zattrs across all layers | ||
| for (y in labels(x)) { | ||
| msg <- c(msg, .validateLabelArray(y)) | ||
| msg <- c(msg, .validateZattrsLabelArray(y)) | ||
| } | ||
| for (y in images(x)) msg <- c(msg, .validateImageArray(y)) | ||
| for (y in points(x)) msg <- c(msg, .validatePointFrame(y)) | ||
| for (y in shapes(x)) msg <- c(msg, .validateShapeFrame(y)) | ||
| msg <- c(msg, .validateTable(x)) | ||
| for (y in labels(x)) .validateZattrsLabelArray(y) | ||
| if (length(msg)) | ||
| return(msg) | ||
| return(TRUE) | ||
| return(msg) | ||
| } | ||
|
|
||
| #' @importFrom S4Vectors setValidity2 | ||
| setValidity2("SpatialData", .validateSpatialData) | ||
|
|
||
| .validateZattrs_multiscales <- \(x, msg) { | ||
| if (is.null(ms <- x$multiscales)) | ||
| if (is.null(ms <- x$multiscales[[1]])) | ||
| msg <- c(msg, "missing 'multiscales'") | ||
| # MUST contain | ||
| for (. in c("axes", "datasets")) | ||
|
|
@@ -96,18 +117,18 @@ setValidity2("SpatialData", .validateSpatialData) | |
| .validateZattrs_coordTrans <- \(x, msg) { | ||
| if (!is.list(ct <- x$coordinateTransformations)) | ||
| msg <- c(msg, "missing or non-list 'coordTrans'") | ||
| ct <- ct[[1]] | ||
| for (. in c("input", "output", "type")) | ||
| if (is.null(ct[[.]])) | ||
| msg <- c(msg, sprintf("'coordTrans' missing '%s'", .)) | ||
| for (i in seq_along(ct)) | ||
| for (j in c("input", "output", "type")) | ||
| if (is.null(ct[[i]][[j]])) | ||
| msg <- c(msg, sprintf("'coordTrans' %s missing '%s'", i, j)) | ||
| return(msg) | ||
| } | ||
| .validateZattrsLabelArray <- \(x) { | ||
| msg <- c() | ||
| za <- meta(x) | ||
| msg <- .validateZattrs_multiscales(za, msg) | ||
| ms <- za$multiscales | ||
| ms <- za$multiscales[[1]] | ||
| msg <- .validateZattrs_axes(ms, msg) | ||
| msg <- .validateZattrs_coordTrans(ms, msg) | ||
| if (length(msg)) return(msg) else return(TRUE) | ||
| return(msg) | ||
|
Comment on lines
126
to
+133
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,48 @@ | ||||||||||||||
| require(dplyr, quietly=TRUE) | ||||||||||||||
| zs <- file.path("extdata", "blobs.zarr") | ||||||||||||||
| zs <- system.file(zs, package="SpatialData") | ||||||||||||||
| sd <- readSpatialData(zs, tables=FALSE) | ||||||||||||||
|
|
||||||||||||||
| test_that("validity,ImageArray", { | ||||||||||||||
| # all resolutions should be numbers | ||||||||||||||
| # (note: logical gets coerced to binary) | ||||||||||||||
| expect_error(ImageArray(list(v <- character(1)))) | ||||||||||||||
| x <- image(sd,1); x@data[[1]][1,1,1] <- v; expect_error(validObject(x)) | ||||||||||||||
| x <- image(sd,2); x@data[[2]][1,1,1] <- v; expect_error(validObject(x)) | ||||||||||||||
| # there should be three dimensions (channels + spatial) | ||||||||||||||
| expect_error(ImageArray(list(a <- array(numeric(1), c(1,1))))) | ||||||||||||||
| x <- image(sd,1); x@data[[1]] <- a; expect_error(validObject(x)) | ||||||||||||||
| x <- image(sd,2); x@data[[2]] <- a; expect_error(validObject(x)) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| test_that("validity,LabelArray", { | ||||||||||||||
| # all resolutions should be of type integer | ||||||||||||||
| for (v in list(logical(1), character(1), numeric(1))) { | ||||||||||||||
| expect_error(LabelArray(list(v))) | ||||||||||||||
| x <- label(sd,1); x@data[[1]][1,1] <- v; expect_error(validObject(x)) | ||||||||||||||
| x <- label(sd,2); x@data[[2]][1,1] <- v; expect_error(validObject(x)) | ||||||||||||||
| } | ||||||||||||||
| # there should be two dimensions | ||||||||||||||
| expect_error(LabelArray(list(a <- array(integer(1), c(1,1,1))))) | ||||||||||||||
| x <- label(sd,1); x@data[[1]] <- a; expect_error(validObject(x)) | ||||||||||||||
| x <- label(sd,2); x@data[[2]] <- a; expect_error(validObject(x)) | ||||||||||||||
| }) | ||||||||||||||
|
|
||||||||||||||
| test_that("validity,PointFrame", { | ||||||||||||||
| x <- point(sd,1) | ||||||||||||||
| expect_error(validObject(select(x, -x))) | ||||||||||||||
| expect_error(validObject(select(x, -y))) | ||||||||||||||
| expect_silent(validObject(select(x, -c(x, y))[0,])) | ||||||||||||||
|
Comment on lines
+33
to
+35
|
||||||||||||||
| expect_error(validObject(select(x, -x))) | |
| expect_error(validObject(select(x, -y))) | |
| expect_silent(validObject(select(x, -c(x, y))[0,])) | |
| expect_error(validObject(dplyr::select(x, -x))) | |
| expect_error(validObject(dplyr::select(x, -y))) | |
| expect_silent(validObject(dplyr::select(x, -c(x, y))[0,])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In
.validateZattrs_coordTrans(), whencoordinateTransformationsis missing or not a list you append an error message but still iterate overctand indexct[[i]][[j]], which can throw for non-list/atomic values. Return early (or wrap the loop in anelse) whencoordinateTransformationsis not a valid list.