Local proximity score - #152
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new analysis function, local_proximity(), to compute per-node Local Neighborhood Enrichment (LNE) scores on PNA CellGraph objects, enabling hot/cold spot detection and marker co-localization scoring.
Changes:
- Introduces
local_proximity()with analytical and permutation-based expected-value computation and multiple scoring modes. - Exports and documents the new API (NAMESPACE + Rd), and wires it into package collation.
- Adds
testthatcoverage for key modes and input validation, and updates CHANGELOG/WORDLIST.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
R/local_proximity_scores.R |
Implements local_proximity() and helper functions for analytical/permutation expectations. |
tests/testthat/test-local_proximity.R |
Adds tests for single/multi-marker scoring, modes, permutation method, and invalid inputs. |
NAMESPACE |
Exports local_proximity. |
man/local_proximity.Rd |
Generated documentation for the new function. |
DESCRIPTION |
Adds new R file to Collate. |
inst/WORDLIST |
Adds “LNE” to spelling whitelist. |
CHANGELOG.md |
Notes the new local_proximity feature. |
Files not reviewed (1)
- man/local_proximity.Rd: Generated file
Suppressed comments (1)
R/local_proximity_scores.R:299
- This negated equality check relies on operator precedence and is hard to read/maintain. Use
mode != "self-clustering"explicitly to avoid ambiguity.
if (!mode == "self-clustering") {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ed4fe9d. Configure here.
| if (mode == "any") { | ||
| freq2 <- freq2 %>% sum() | ||
| } | ||
| local_stat_exp_B <- freq2 * degree_exp_B |
There was a problem hiding this comment.
Wrong analytical null for all mode
High Severity
For mode = "all" with multiple markers, the analytical expected value calculation in local_proximity is inconsistent with the observed statistic. The expected value uses the minimum of marker frequencies per UMI type, while the observed statistic takes the minimum of per-marker neighborhood sums. This mismatch biases analytical LNE scores and prevents the permutation method from converging to the analytical result as intended.
Reviewed by Cursor Bugbot for commit ed4fe9d. Configure here.
There was a problem hiding this comment.
Hmm, is this something to look into?
| ) | ||
|
|
||
| # Compute log2 ratio of observed to expected statistic | ||
| log2_ratio <- log2(pmax(local_stat_obs, k - 1) / pmax(local_stat_exp, k - 1)) |
There was a problem hiding this comment.
Ratio floor scales with neighborhood k
Medium Severity
The local_proximity function applies a k-dependent floor (pmax(..., k - 1)) to both observed and expected statistics before calculating the log2 ratio. This deviates from the documented log2(O/E) formula, materially altering scores and potentially flattening weak signals. This can lead to misinterpretation of magnitudes and incorrect comparisons.
Reviewed by Cursor Bugbot for commit ed4fe9d. Configure here.
| local_stat_exp_A <- outer(degree_exp_A, freq1) | ||
| } | ||
| if (!exists("freq2")) { | ||
| freq2 <- (counts[(nA + 1):(nA + nB), , drop = FALSE] %>% Matrix::colSums()) / nB |
There was a problem hiding this comment.
Frequency lookup escapes function scope
Medium Severity
.analytical_lp_exp uses exists("freq1") and exists("freq2") with default inherits = TRUE. If a caller has freq1 or freq2 in the global environment, frequency computation from counts is skipped and those external values are used silently, corrupting analytical LNE scores.
Reviewed by Cursor Bugbot for commit ed4fe9d. Configure here.
Description
Added functionality to calculate Local Proximity Scores per each node of a graph. This can be used to calculate hot/cold spots for a given marker, or visaulize regions where multiple markers colocalize to a higher degree than expected, given their abundance.
Added
local_proximityto compute Local Neighborhood Enrichment (LNE)log2 ratios of observed vs expected UMI counts for selected marker(s).
Type of change
How Has This Been Tested?
Tests have been added.
PR checklist:
Note
Medium Risk
New graph statistics code with non-trivial null models and matrix ops on large subgraphs; risk is moderate due to numerical/assumption sensitivity and runtime for permutation mode, not due to security or data-handling paths.
Overview
Adds exported
local_proximityfor per-node Local Neighborhood Enrichment (LNE) onCellGraphobjects: log2 ratios of observed vs expected marker UMI counts within ak-hop neighborhood (via existingexpand_adjacency_matrix), with separate handling for PNAumi1/umi2node types.Computation options:
modeall(joint colocation via min counts),any(sum across markers), orself-clustering(per-marker matrix, analytical only);methodanalytical(frequency × neighborhood size) orpermutation(label shuffle null, converges with moreiterations). Optional precomputed reachability matrixA_k, tunablek(2–10), and seed for permutations.Package wiring: new
R/local_proximity_scores.R, roxygen/man, CHANGELOG,LNEin wordlist, and testthat coverage (marker sets, modes, permutation, invalid inputs).Reviewed by Cursor Bugbot for commit ed4fe9d. Bugbot is set up for automated code reviews on this repo. Configure here.