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.

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

Fix SpatiallyVariableFeatures.Assay & Introduce SVFInfo/SpatiallyVariableFeatures.StdAssay #242

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,19 @@ S3method(ReorderIdent,Seurat)
S3method(S4ToList,default)
S3method(S4ToList,list)
S3method(SVFInfo,Assay)
S3method(SVFInfo,Assay5)
S3method(SVFInfo,Seurat)
S3method(SVFInfo,StdAssay)
S3method(SetAssayData,Assay)
S3method(SetAssayData,Seurat)
S3method(SetAssayData,StdAssay)
S3method(SetIdent,Seurat)
S3method(Simplify,Molecules)
S3method(Simplify,Spatial)
S3method(SpatiallyVariableFeatures,Assay)
S3method(SpatiallyVariableFeatures,Assay5)
S3method(SpatiallyVariableFeatures,Seurat)
S3method(SpatiallyVariableFeatures,StdAssay)
S3method(StashIdent,Seurat)
S3method(Stdev,DimReduc)
S3method(Stdev,Seurat)
Expand Down
12 changes: 6 additions & 6 deletions R/assay.R
Original file line number Diff line number Diff line change
Expand Up @@ -878,9 +878,9 @@ SpatiallyVariableFeatures.Assay <- function(
method <- selection.method
}
vf <- SVFInfo(object = object, method = method, status = TRUE)
vf <- vf[rownames(x = vf)[which(x = vf[, "variable"][, 1])], ]
vf <- vf[rownames(x = vf)[which(!is.na(x = vf[, "variable"]))], ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you revert this change, then I think you can just return rownames(vf) without sorting again. I think this was your intent in SVFInfo.StdAssay except that updating this line means that features where variable == FALSE will still be returned.

Suggested change
vf <- vf[rownames(x = vf)[which(!is.na(x = vf[, "variable"]))], ]
vf <- vf[rownames(vf)[which(vf[, "variable"][, 1])], ]

if (!is.null(x = decreasing)) {
vf <- vf[order(x = vf[, "rank"], decreasing = !decreasing), ]
vf <- vf[order(x = vf[, "rank"][, 1], decreasing = !decreasing), ]
}
return(rownames(x = vf)[which(x = vf[, "variable"][, 1])])
}
Expand Down Expand Up @@ -911,12 +911,12 @@ SVFInfo.Assay <- function(
EXPR = method,
markvariogram = grep(
pattern = "r.metric",
x = colnames(x = object[]),
x = colnames(x = object[[]]),
value = TRUE
),
moransi = grep(
pattern = 'moransi',
x = colnames(x = object[]),
pattern = 'MoransI',
x = colnames(x = object[[]]),
value = TRUE
),
abort(message = paste("Unknown method:", sQuote(x = method)))
Expand All @@ -925,7 +925,7 @@ SVFInfo.Assay <- function(
expr = svf.info <- object[[vars]],
error = function(e) {
stop(
"Unable to find highly variable feature information for method '",
"Unable to find spatially variable feature information for method '",
method,
"'",
call. = FALSE
Expand Down
96 changes: 96 additions & 0 deletions R/assay5.R
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,102 @@ VariableFeatures.Assay5 <- VariableFeatures.StdAssay
#'
"VariableFeatures<-.Assay5" <- `VariableFeatures<-.StdAssay`

#' @rdname VariableFeatures
#' @export
#' @method SVFInfo StdAssay
#'
SVFInfo.StdAssay <- function(
object,
method = c("markvariogram", "moransi"),
status = FALSE,
selection.method = deprecated(),
...
) {
CheckDots(...)
if (is_present(arg = selection.method)) {
.Deprecate(
when = '5.0.0',
what = 'SVFInfo(selection.method = )',
with = 'SVFInfo(method = )'
)
method <- selection.method
}
method <- match.arg(arg = method)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! You should drop the redundant method <- method[1] call from SVFInfo.Assay too 👌

vars <- switch(
EXPR = method,
markvariogram = grep(
pattern = "r.metric",
x = colnames(x = object[[]]),
value = TRUE
),
moransi = grep(
pattern = 'MoransI',
x = colnames(x = object[[]]),
value = TRUE
),
abort(message = paste("Unknown method:", sQuote(x = method)))
)

tryCatch(
expr = svf.info <- object[[vars]],
error = function(e) {
stop(
"Unable to find spatially variable feature information for method '",
method,
"'",
call. = FALSE
)
}
)

colnames(x = svf.info) <- vars
if (status) {
svf.info$variable <- object[[paste0(method, '.spatially.variable')]]
svf.info$rank <- object[[paste0(method, '.spatially.variable.rank')]]
}
return(svf.info)
}
Comment on lines +1694 to +1744
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SVFInfo.StdAssay <- function(
object,
method = c("markvariogram", "moransi"),
status = FALSE,
selection.method = deprecated(),
...
) {
CheckDots(...)
if (is_present(arg = selection.method)) {
.Deprecate(
when = '5.0.0',
what = 'SVFInfo(selection.method = )',
with = 'SVFInfo(method = )'
)
method <- selection.method
}
method <- match.arg(arg = method)
vars <- switch(
EXPR = method,
markvariogram = grep(
pattern = "r.metric",
x = colnames(x = object[[]]),
value = TRUE
),
moransi = grep(
pattern = 'MoransI',
x = colnames(x = object[[]]),
value = TRUE
),
abort(message = paste("Unknown method:", sQuote(x = method)))
)
tryCatch(
expr = svf.info <- object[[vars]],
error = function(e) {
stop(
"Unable to find spatially variable feature information for method '",
method,
"'",
call. = FALSE
)
}
)
colnames(x = svf.info) <- vars
if (status) {
svf.info$variable <- object[[paste0(method, '.spatially.variable')]]
svf.info$rank <- object[[paste0(method, '.spatially.variable.rank')]]
}
return(svf.info)
}
SVFInfo.Assay5 <- SVFInfo.StdAssay


#' @rdname SpatiallyVariableFeatures-StdAssay
#' @method SpatiallyVariableFeatures StdAssay
#' @export
#'
SpatiallyVariableFeatures.StdAssay <- function(
object,
method = "moransi",
decreasing = TRUE,
selection.method = deprecated(),
...
) {
CheckDots(...)
if (is_present(arg = selection.method)) {
.Deprecate(
when = '5.0.0',
what = 'SpatiallyVariableFeatures(selection.method = )',
with = 'SpatiallyVariableFeatures(method = )'
)
method <- selection.method
}
vf <- SVFInfo(object = object, method = method, status = TRUE)
vf <- vf[rownames(x = vf)[which(!is.na(x = vf[, "variable"]))], ]
if (!is.null(x = decreasing)) {
vf <- vf[order(x = vf[, "rank"][, 1], decreasing = !decreasing), ]
}
return(rownames(x = vf))
}
Comment on lines +1750 to +1772
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
SpatiallyVariableFeatures.StdAssay <- function(
object,
method = "moransi",
decreasing = TRUE,
selection.method = deprecated(),
...
) {
CheckDots(...)
if (is_present(arg = selection.method)) {
.Deprecate(
when = '5.0.0',
what = 'SpatiallyVariableFeatures(selection.method = )',
with = 'SpatiallyVariableFeatures(method = )'
)
method <- selection.method
}
vf <- SVFInfo(object = object, method = method, status = TRUE)
vf <- vf[rownames(x = vf)[which(!is.na(x = vf[, "variable"]))], ]
if (!is.null(x = decreasing)) {
vf <- vf[order(x = vf[, "rank"][, 1], decreasing = !decreasing), ]
}
return(rownames(x = vf))
}
SpatiallyVariableFeatures.StdAssay <- SpatiallyVariableFeatures.Assay


#' @rdname SpatiallyVariableFeatures
#' @method SpatiallyVariableFeatures Assay5
#' @export
#'
SpatiallyVariableFeatures.Assay5 <- SpatiallyVariableFeatures.StdAssay

#' @rdname VariableFeatures
#' @method SVFInfo Assay5
#' @export
#'
SVFInfo.Assay5 <- SVFInfo.StdAssay

Comment on lines +1774 to +1785
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit—consistent ordering:

Suggested change
#' @rdname SpatiallyVariableFeatures
#' @method SpatiallyVariableFeatures Assay5
#' @export
#'
SpatiallyVariableFeatures.Assay5 <- SpatiallyVariableFeatures.StdAssay
#' @rdname VariableFeatures
#' @method SVFInfo Assay5
#' @export
#'
SVFInfo.Assay5 <- SVFInfo.StdAssay
#' @rdname VariableFeatures
#' @method SVFInfo Assay5
#' @export
#'
SVFInfo.Assay5 <- SVFInfo.StdAssay
#' @rdname SpatiallyVariableFeatures
#' @method SpatiallyVariableFeatures Assay5
#' @export
#'
SpatiallyVariableFeatures.Assay5 <- SpatiallyVariableFeatures.StdAssay

#' @method WhichCells StdAssay
#' @export
#'
Expand Down