Missing `rpy2` (optional) dependency?

[mschilli87]

Please make sure these conditions are met

• I have checked that this issue has not already been reported.
• I have confirmed this bug exists on the latest version of pertpy.
• (optional) I have confirmed this bug exists on the main branch.

Report

pertpy.tl.Milo().da_nhoods (conditionally) imports from rpy2:

pertpy/pertpy/tools/_milo.py

Lines 357 to 360 in cdccdb0

	import rpy2.robjects as ro
	from rpy2.robjects import numpy2ri, pandas2ri
	from rpy2.robjects.conversion import localconverter
	from rpy2.robjects.vectors import FloatVector

However, this (optional) dependency is not specified in pyproject.toml (and thus not installed by the Conda recipe).

Versions

[Zethson]

It does but only if the solver is edger (see just a few lines above). This is expected behavior, right?

We are mentioning rpy2 here: https://github.com/scverse/pertpy?tab=readme-ov-file#milo but I see that this won't translate into the Conda recipe. We could consider adding the milo group again?

[mschilli87]

@Zethson:

It does but only if the solver is edger (see just a few lines above).

That's why I uses 'conditionally'/'optional'.
I think adding a milo group for this dependency would be a good solution.
That could be carried over to conda as well.

[Zethson]

Ah, I missed that, sorry.

I'm only partially a fan of an extra named milo because it could imply that milo doesn't work if you don't have that extra installed. edger-milo or something like this looks stupid though. 🤔

Are you willing to make a quick PR?

[mschilli87]

Yes. Can do so tomorrow in the morning.

[mschilli87]

@Zethson: The PR for PyPi/pip is in. However, to get full 'out-of-the-box' support for milo via Conda, we'd have to not only add rpy2, but also edgeR to the recipe. Installing edgeR via conda requires the bioconda channel as it's packaged by BioConductor as bioconductor-edger. I did a quick check and found that bioconda packages can depend of conda-forge, but could not find any information supporting the inverse statement. Do you have any definite information on that? And if my assessemnt it correct, would you be open to moving pertpy from the conda-forge to the bioconda channel for an optional dependency? Or would you prefer documenting somewhere (and if so: where?) clearly that running milo with the (default!) edger solver from a Conda install requires the manual specification of the bioconda channel and the bioconductor-edger dependency in addition to pertpy from the conda-forge channel?

[Zethson]

would you be open to moving pertpy from the conda-forge to the bioconda channel for an optional dependency?

No. We actually moved all scverse packages from bioconda to conda-forge at some point. I can't find the related discussion at them moment but it's in some scanpy issue.

but could not find any information supporting the inverse statement. Do you have any definite information on that?

It's indeed not possible. Users have to add the bioconda channel themselves to also install edger. IMO this is a big shortcoming of Conda but sadly nothing that we can reasonably solve.

Or would you prefer documenting somewhere (and if so: where?) clearly that running milo with the (default!) edger solver from a Conda install requires the manual specification of the bioconda channel and the bioconductor-edger dependency in addition to pertpy from the conda-forge channel?

Yes! Let's do this.