Hi SpatialCellChat Team— while reading through the source (with the help of an AI-assisted code-review pass) I ran into a handful of places where what the function does seems to disagree with what its parameters / docstring promise. Listing them below with file/line references in case any are already known or intentional. Hope that it can be helpful in anyway.
1. normalizeData(do.log = FALSE) returns an undefined data.norm
Location: R/utilities.R line 787 (function body)
normalizeData <- function(data.raw, scale.factor = 10000, do.log = TRUE) {
...
expr <- Matrix::t(Matrix::t(data.raw) / library.size) * scale.factor
if (do.log) {
data.norm <- log1p(expr)
}
return(data.norm) # never assigned when do.log = FALSE
}
When do.log = FALSE, data.norm is never bound, so the function errors with Error: object 'data.norm' not found. Given the parameter doc is "whether to apply log transformation", the FALSE branch presumably should return the un-logged expr:
data.norm <- if (do.log) log1p(expr) else expr
return(data.norm)
2. sketchData references an undefined X
Location: R/utilities.R around line 898 (non-Seurat branch of sketchData; same code is in upstream CellChat::utilities.R:111)
if (do.PCA) {
X.pcs <- runPCA(object, dimPC = dimPC)
} else {
X.pcs <- object
}
# Sketch percent of data.
sketch.size <- as.integer(percent * nrow(X)) # X undefined here
X is not in scope — only X.pcs and object exist — so the function aborts with Error: object 'X' not found. The comment "Sketch percent of data" and the surrounding logic suggest the intent was nrow(object):
sketch.size <- as.integer(percent * nrow(object))
3. selectK ignores k.range/nrun and references an undefined data_sr
Location: R/analysis.R line 962
res <- NMF::nmfEstimateRank(data_sr, range = 20:30, nrun = 30L, seed = seed.use)
Three issues at once:
data_sr is undefined in the function body — only data / data0 exist — so the call errors with Error: object 'data_sr' not found.
range = 20:30 is hardcoded, ignoring the function's own k.range argument.
nrun = 30L is hardcoded, ignoring the function's own nrun argument.
For comparison, the analogous line in upstream jinworks/CellChat (analysis.R:320) reads:
res <- NMF::nmfEstimateRank(data, range = k.range, method = 'lee',
nrun = nrun, seed = seed.use)
— which would resolve all three points if applied here.
4. suggestK1 silently drops its documented L1 argument
Location: R/analysis.R lines 3772-3779, and again at lines 3815-3822 (the parallel branch)
The signature documents:
#' @param L1 L1/LASSO penalties between 0 and 1, array of length two for c(w, h)
but the body calls
outs_NMF <- RcppML::nmf(
Mat, k = k, tol = tol, maxit = maxit,
seed = seedSample[[i]], verbose = F
)
— L1 is never forwarded to RcppML::nmf, so whatever the user passes is silently ignored. Both call sites would need L1 = L1 to honour the documented behaviour.
Hi SpatialCellChat Team— while reading through the source (with the help of an AI-assisted code-review pass) I ran into a handful of places where what the function does seems to disagree with what its parameters / docstring promise. Listing them below with file/line references in case any are already known or intentional. Hope that it can be helpful in anyway.
1.
normalizeData(do.log = FALSE)returns an undefineddata.normLocation:
R/utilities.Rline 787 (function body)When
do.log = FALSE,data.normis never bound, so the function errors withError: object 'data.norm' not found. Given the parameter doc is "whether to apply log transformation", theFALSEbranch presumably should return the un-loggedexpr:2.
sketchDatareferences an undefinedXLocation:
R/utilities.Raround line 898 (non-Seurat branch ofsketchData; same code is in upstreamCellChat::utilities.R:111)Xis not in scope — onlyX.pcsandobjectexist — so the function aborts withError: object 'X' not found. The comment "Sketch percent of data" and the surrounding logic suggest the intent wasnrow(object):3.
selectKignoresk.range/nrunand references an undefineddata_srLocation:
R/analysis.Rline 962Three issues at once:
data_sris undefined in the function body — onlydata/data0exist — so the call errors withError: object 'data_sr' not found.range = 20:30is hardcoded, ignoring the function's ownk.rangeargument.nrun = 30Lis hardcoded, ignoring the function's ownnrunargument.For comparison, the analogous line in upstream
jinworks/CellChat(analysis.R:320) reads:— which would resolve all three points if applied here.
4.
suggestK1silently drops its documentedL1argumentLocation:
R/analysis.Rlines 3772-3779, and again at lines 3815-3822 (the parallel branch)The signature documents:
#' @param L1 L1/LASSO penalties between 0 and 1, array of length two for c(w, h)but the body calls
—
L1is never forwarded toRcppML::nmf, so whatever the user passes is silently ignored. Both call sites would needL1 = L1to honour the documented behaviour.