omerwe / polyfun

PolyFun (POLYgenic FUNctionally-informed fine-mapping)
MIT License
85 stars 21 forks source link

Loosen rpy2 pin #156

Closed jdblischak closed 1 year ago

jdblischak commented 1 year ago

This is a follow up to PR #149. I updated finemapper.py to use the existing rpy2 code if rpy2 <3.5.9, and to use the solution from @Ojami in https://github.com/omerwe/polyfun/pull/149#issuecomment-1484110637 if rpy2 >=3.5.9.

As a precaution to avoid the potential missing value bug in rpy2 3.5.7 and 3.5.8, I updated the polyfun.yml file to avoid these 2 versions. However, since I don't actually have any evidence that the bug will affect the polyfun code, I didn't explicitly throw an error in finemapper.py if one of those versions is installed. In other words, I'm directing users away from these versions, but not preventing a user if they for some reason really want to install one of those versions.

I compare the version strings using the 3rd party package packaging. This package is maintained by the PyPA, is a dependency of setuptools, and was already installed in the existing polyfun env. Thus I added it to polyfun.yml for documentation purposes, but polyfun.yml.lock doesn't need to be updated.

xref: #153

jdblischak commented 1 year ago

The updated code worked with rpy2 3.5.6 in the locked env and rpy2 3.5.11 in the unrestricted env

jdblischak commented 1 year ago

And I also tested locally with rpy2 3.4.5

omerwe commented 1 year ago

Looks awesome, thanks as always @jdblischak!