Skip to content

Fix highly variable gene selection with duplicate names - #4417

Open
thirumurugan-2006 wants to merge 4 commits into
scverse:mainfrom
thirumurugan-2006:fix/4405-duplicate-gene-names
Open

thirumurugan-2006 wants to merge 4 commits into
scverse:mainfrom
thirumurugan-2006:fix/4405-duplicate-gene-names

Conversation

@thirumurugan-2006

@thirumurugan-2006 thirumurugan-2006 commented Oct 12, 2026 •

Copy link
Copy Markdown

<!--
Thanks for opening a PR to scanpy!
Please be sure to follow the guidelines in our contribution guide (https://scanpy.scverse.org/page/dev/) to familiarize yourself with our workflow and speed up review.
-->

LLM usage

Summary

scanpy.pp.highly_variable_genes(flavor="seurat_v3") could select more than n_top_genes when adata.var_names contained duplicate gene names.

The previous implementation used label-based .loc assignment after sorting. Duplicate labels could cause multiple rows to be marked as highly variable, exceeding the requested number.

This change uses positional row selection for the final assignment, ensuring that selection is based on individual rows rather than gene-name labels.

Tests

  • Added regression coverage for unique and duplicate gene names.
  • Verified that both cases select exactly 30 highly variable genes when n_top_genes=30.
  • Ran the relevant seurat_v3 and selection-consistency tests.
  • Ran Ruff linting and formatting checks.
  • Ran git diff --check and Python syntax checks.

Release notes

No release note is necessary because this is a focused bug fix with no public API change.

@thirumurugan-2006

Copy link
Copy Markdown
Author

can i merge it

@codecov

codecov Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (0f9179e) to head (450357a).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #4417       +/-   ##
==========================================
- Coverage   83.78%       0   -83.79%     
==========================================
  Files         134       0      -134     
  Lines       12691       0    -12691     
==========================================
- Hits        10633       0    -10633     
+ Misses       2058       0     -2058     
Flag Coverage Δ
hatch-test.low-vers ?
hatch-test.pre ?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 134 files with indirect coverage changes

@thirumurugan-2006

Copy link
Copy Markdown
Author

is the benchmark will take longer time

thirumurugan-2006 and others added 2 commits October 12, 2026 19:44
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@thirumurugan-2006

Copy link
Copy Markdown
Author

The failing CI run appears to be unrelated to my documentation changes. The failure is in tests/test_preprocessing_distributed.py::test_write_zarr, where an AnnData/Dask/Zarr auto-sharding warning is promoted to an error. Since this run is two weeks old, could you confirm whether the issue persists on the current branch or whether the latest CI result can be used for merge eligibility?

This branch has not been deployed

No deployments
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.

highly_variable_genes(flavor="seurat_v3") flags more than n_top_genes genes when var_names contain duplicates

1 participant