Skip to content

A few function bodies appear to disagree with their docs / signatures #12

@jefferyUstc

Description

@jefferyUstc

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:

  1. data_sr is undefined in the function body — only data / data0 exist — so the call errors with Error: object 'data_sr' not found.
  2. range = 20:30 is hardcoded, ignoring the function's own k.range argument.
  3. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions