Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use_rep in cluster_optimal_resolution and opt_louvain #389

Open
kzkedzierska opened this issue Nov 5, 2023 · 0 comments
Open

use_rep in cluster_optimal_resolution and opt_louvain #389

kzkedzierska opened this issue Nov 5, 2023 · 0 comments

Comments

@kzkedzierska
Copy link

Hiya!

First off, thanks a ton for this awesome package – it’s super helpful!

There seems to be some inconsistency across those two functions: cluster_optimal_resolution and opt_louvain. In opt_louvain, even if use_rep is specified when adata.uns["neighbors"] is already present, the embedding argument is ignored (here). However, in the case of cluster_optimal_resolution the function doesn't look for adata.uns["neighbors"] when use_rep is specified (here).

I noticed that opt_louvain is deprecated, but I think it would be useful to raise a warning when use_rep is used while adata.uns["neighbors"] is already there. The docs do mention that use_rep gets ignored in that scenario, but it still escaped me and caused me a bit of a headache. I thought that maybe a warning could save the next person.

On another note - in the case of cluster_optimal_resolution, if I'm correct, the docstring mentioning "only if adata.uns['neighbors'] is not defined, otherwise will be ignored" might be a bit misleading.

If you are ok with it and it would be of any help I can submit a PR.

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

No branches or pull requests

1 participant