theislab / ehrapy

Electronic Health Record Analysis with Python.
https://ehrapy.readthedocs.io/
Apache License 2.0
199 stars 19 forks source link

fix ep.tl.umap when neighbors_key is not None #790

Closed eroell closed 1 month ago

eroell commented 1 month ago

PR Checklist

Description of changes Fix a bug blocking the native the use of ep.tl.umap when ep.pp.neighbors(adata, key_added=<custom>).

Technical details Simply remove a faulty check & let scanpy handle it.

Adding tests for plotting and the way to there are an open issue #666, and will be added another time more comprehensively,

Additional context

eroell commented 1 month ago

@VladimirShitov if you want to quickly review (or particularly if you have a dev install of ehrapy, confirm this branch now works as it should for you), much appreciated :) else I'll bug Lukas ;)

Zethson commented 1 month ago

@eroell does scanpy suggest to run scanpy neighbors or just calculate neighbors in general? This is legacy code to suggest people to calculate neighbors using the ehrapy API

VladimirShitov commented 1 month ago

I don't have dev install, but the test case from the issue works with scanpy so should be fine now

eroell commented 1 month ago

@eroell does scanpy suggest to run scanpy neighbors or just calculate neighbors in general? This is legacy code to suggest people to calculate neighbors using the ehrapy API

you're right ofc it instructs to use sc.pp.neighbors. added the check again, this time with the proper logic