Skip to content

Removed presto from DifferentialProximityAnalysis - #165

Merged
ludvigla merged 3 commits into
mainfrom
dat-124-remove-presto
Sep 4, 2026
Merged

Removed presto from DifferentialProximityAnalysis#165
ludvigla merged 3 commits into
mainfrom
dat-124-remove-presto

Conversation

@ludvigla

@ludvigla ludvigla commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR removes the option to use presto for computing Wilcoxon Rank Sum Tests in DifferentialProximityAnalysis. The reason is that presto::wilcoxauc doesn't handle negative values correctly.

Now the default is limma, but falls back to base R wilcox.test if limma is not installed.

Fixes: DAT-124

Type of change

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • This change requires a documentation update.

How Has This Been Tested?

Updated tests for RunDAA and DifferentialProximityAnalysis. These no longer expect the presto backend.

PR checklist:

  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have documented any significant changes to the code in CHANGELOG.md

Note

Medium Risk
Changes the statistical engine for Wilcoxon tests in differential proximity (and related) workflows, so p-values can differ from prior releases when presto was used; the fix targets correctness for negative scores.

Overview
Bumps the package to v0.21.0 and documents the change in CHANGELOG.md and the README beta badge.

Wilcoxon rank-sum testing in matrix-based differential proximity analysis (.wilcox_de_test, used by DifferentialProximityAnalysis.Matrix / Seurat "seurat" method) no longer calls presto::wilcoxauc, which mishandled negative proximity values. The path is now limma::rankSumTestWithCorrelation when limma is installed, otherwise stats::wilcox.test per feature, with the install hint updated from presto to limma.

Dependencies: presto and its GitHub Remotes entry are removed from DESCRIPTION. limma remains a Suggests dependency only.

Tests refresh expected p / p_adj values in test-DifferentialProximityAnalysis.R and test-RunDAA.R to match the new test backend (comments about CI expecting presto are dropped).

Reviewed by Cursor Bugbot for commit d117b25. Bugbot is set up for automated code reviews on this repo. Configure here.

@ludvigla
ludvigla requested a review from maxkarlsson September 4, 2026 10:38

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 212417c. Configure here.

Comment thread tests/testthat/test-RunDAA.R
…sto uses a sligthly different formula for computing p-values.

@maxkarlsson maxkarlsson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks Good!

@ludvigla
ludvigla merged commit 93e1120 into main Sep 4, 2026
8 checks passed
@ludvigla
ludvigla deleted the dat-124-remove-presto branch September 4, 2026 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants