Fix SpatiallyVariableFeatures.Assay & Introduce SVFInfo/SpatiallyVariableFeatures.StdAssay

[zskylarli]

This PR fixes a small bug with SpatiallyVariableFeatures.Assay and then introduces implementations for SpatiallyVariableFeatures.StdAssay/SpatiallyVariableFeatures.Assay5 & SVFInfo.StdAssay/SVFInfo.Assay5.

The error was being thrown by this call to order—apparently it has always been wrong to call order on a data.frame but it became an error in R 4.3.0.

[dcollins15]

Very nice! 🙌

Ideally, we'd like to implement SVFInfo.StdAssay & SpatiallyVariableFeatures.StdAssay to emulate the interfaces for HVFInfo.StdAssay and SpatiallyVariableFeatures.StdAssay so that they expose a layers argument. Since this would require updating Seurat::FindSpatiallyVariableFeatures.StdAssay to be layer-aware I'm ok with just duplicating the Assay implementations for now.

In that case, I think it'll be a lot cleaner to make the methods explicitly equivalent—I've added some suggested changes to that effect.

---

Please also keep in mind that any links to to lab's private repositories (e.g. satijalab/seurat-private#138) cannot be viewed by external contributors and should be avoided in public-facing PRs. I've updated the description for this PR to use as an example going forward.

Thanks!

[dcollins15]

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

[dcollins15]

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

[dcollins15]

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

[dcollins15]

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

[dcollins15]

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])], ]