Skip to content

Include equal distances in permutation-test upper tails - #1094

Closed
dnncha wants to merge 1 commit into
scverse:mainfrom
dnncha:fix/permutation-inclusive-ties
Closed

Include equal distances in permutation-test upper tails#1094
dnncha wants to merge 1 commit into
scverse:mainfrom
dnncha:fix/permutation-inclusive-ties

Conversation

@dnncha

@dnncha dnncha commented Sep 8, 2026

Copy link
Copy Markdown

When all label permutations tie the observed distance, DistanceTest currently reports p=1/n_perms instead of 1. This patch counts equal distances in the upper tail and updates the helper's docstring. It preserves the existing group-label alignment fix and minimum-count convention.

For eight observations split 4/4, both identical nonzero embeddings and an embedding with seven rows (1,1,1) plus one row (9,1,1) have the same statistic for all 70 balanced assignments. The latter has Euclidean distance 2 and E-distance 1 for every assignment. Released Pertpy 1.3.0 gives raw p=0.001 with 1,000 permutations; the inclusive-tail correction gives 1. The same strict comparison remains in current upstream base 3001be8.

Minimal public-API reproduction:

import numpy as np
import pandas as pd
from anndata import AnnData
import pertpy as pt
x = np.ones((8, 3))
x[-1, 0] = 9
a = AnnData(x, obs=pd.DataFrame(
    {'group': ['control'] * 4 + ['treated'] * 4,
    }, index=[str(i) for i in range(8)]))
a.obsm['X_test'] = x.copy()
print(pt.tl.DistanceTest('euclidean', n_perms=1000, obsm_key='X_test')(
    a, groupby='group', contrast='control', show_progressbar=False))

Seven new checks cover four public-API fixture/metric combinations, a mixed equal/lower tail, and strictly greater/lower controls. Including the existing group-label alignment regression: before, five fail and three pass; after, all eight pass. The other 18 distance tests use a downloaded dataset and were deselected; the full suite was not run. Changed source/tests pass Ruff lint and formatting checks.

The change addresses exact ties. It does not introduce a floating-point near-tie tolerance, randomized tie-breaking, or revise the general Monte Carlo numerator/denominator convention. No affected published conclusion is established. The inclusive upper-tail convention is also documented by https://docs.scipy.org/doc/scipy/reference/generated/scipy.stats.permutation_test.html .

Published reproduction, full enumeration, released-wheel before/after outputs and evidence archive: https://cheerfulduck.com/research/audits/pertpy-permutation-ties . Prepared with AI assistance; independent human scientific review and a released fix are not claimed.

  • Regression tests added
  • Affected helper documentation updated
  • No matching existing issue found in bounded searches; this PR is the initial upstream report.

@Zethson

Zethson commented Sep 8, 2026

Copy link
Copy Markdown
Member

AI generated slop. If you didn't review as a human, why should I?

@Zethson Zethson closed this Sep 8, 2026
@codecov-commenter

codecov-commenter commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.94%. Comparing base (3001be8) to head (1e7a53c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1094      +/-   ##
==========================================
- Coverage   79.95%   79.94%   -0.02%     
==========================================
  Files          55       55              
  Lines        7533     7533              
==========================================
- Hits         6023     6022       -1     
- Misses       1510     1511       +1     
Files with missing lines Coverage Δ
src/pertpy/tools/_distances/_distance_tests.py 92.74% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dnncha

dnncha commented Sep 8, 2026

Copy link
Copy Markdown
Author

Pertpy can raise a false alarm. All 70 possible groupings give exactly the same result—nothing makes the original special. Yet it reports statistical significance: p=0.001 instead of p=1.

Counting ties correctly fixes these examples. Who found the bug does not change the arithmetic. If the example is wrong, show why. Otherwise, please track and fix it.

Reproducer and evidence.

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.

3 participants