cgat-developers / cgat-flow

cgat-flow repository
MIT License
13 stars 9 forks source link

cgat-flow-pipelines.yml causes installation of cgat-apps 0.6.0 #143

Open IanSudbery opened 2 years ago

IanSudbery commented 2 years ago

if you build an environment based on cgat-flow-pipelines.yml, you'll end up with something that has cgat-apps version 0.6.0 installed. If you edit the yml to force cgat-apps 0.6.4, you end up with a conflict and the environment will fail to solve. I am investigating what might be causing this.

IanSudbery commented 2 years ago

This is because cgat-apps 0.6.4 requires python 3.8 (or at least the conda package states it does), but cgat-flow pins rpy2 <3, and rpy2 <3 is not compatible with python 3.8.

jscaber commented 2 years ago

I have removed quite a bit of rpy2 (specifically from the diffexpression pipeline, which unfortunately is hard coded for human data at present), but it is still present in other pipelines I don’t use, so didn’t feel I had the urgency or expertise to do that. There is a new version of rpy2 which would work but requires rewriting all the code because they completely rewrote the classes to deal with some of the systematic issues of rpy2 before v3. Hope this is of some help. Jakub

On 27 Aug 2021, at 11:12, Ian Sudbery @.**@.>> wrote:

This is because cgat-apps 0.6.4 requires python 3.8 (or at least the conda package states it does), but cgat-flow pins rpy2 <3, and rpy2 <3 is not compatible with python 3.8.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/cgat-developers/cgat-flow/issues/143#issuecomment-907090755, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC4Y5MTA62JTA5B26ROLRC3T65QI3ANCNFSM5C3YLQ7A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

IanSudbery commented 2 years ago

So, a coupled question is why cgat-apps is only built for python >=3.8, because otherwise, cgat-apps and rpy2 2.9 could co-exist.

IanSudbery commented 2 years ago

@jscaber Do you know which things won't work with rpy2 3.1 and if there is a way to test them? I've managed to get an installation working by upping the pin to 3.1, and also removing the pin on pysam.

jscaber commented 2 years ago

It would be the testing pipeline, but again that has not been kept up to date, afaik. I will devote time to it again in future, but have had to return to the wet lab.

I believe it’s about 3 or 4 pipelines directly, forgot off the top of my head, but I think peakcalling, exome and others. Main problem is that rpy2 is also deeply integrated into expression.py and expression_runner.py which both contain quite unwieldy classes (built on Top of the mapping class) that are used widely by other pipelines. I’ve managed to break that dependency for diff expression but not for the others.

Sent from my iPhone

On 27 Aug 2021, at 13:02, Ian Sudbery @.***> wrote:



@jscaberhttps://github.com/jscaber Do you know which things won't work with rpy2 3.1 and if there is a way to test them? I've managed to get an installation working by upping the pin to 3.1, and also removing the pin on pysam.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/cgat-developers/cgat-flow/issues/143#issuecomment-907150374, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC4Y5MX3H3C3JOPOBJTYIZTT655GBANCNFSM5C3YLQ7A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jscaber commented 2 years ago

a pragmatic approach here would be to remove the general rpy2 pin as well, given that the one rpy2 pipeline that works uses rpy2 >3 (rnaseqqc). For the remaining pipelines like peakcalling etc, it would mean greating rpy2 <3 envs or troubleshooting which they all need anyways because they were written so long ago. @Acribbs, what do you think?

jscaber commented 2 years ago

I've had a look at this @Acribbs :

With rpy2 pinned the following are installed: cgat-apps 0.6.4 py37h80fa574_1 bioconda pysam 0.16.0.1 py37h45aed0b_3 bioconda rpy2 2.9.4 py37r36hd767a1f_3 conda-forge and it builds ok.

If I unpin rpy2: rpy2 3.4.5 py37r41h6f94858_1 conda-forge pysam 0.17.0 py37h45aed0b_0 bioconda cgat-apps 0.6.4 py37h80fa574_1 bioconda It stalls and I get following AttributeError: module 'pysam.libcalignmentfile' has no attribute 'PileupColumn' again.

I wonder if it the cimports need fixing on cgat-apps fixing, but am not sure here. Strange that it works with pysam 16 but not 17, I thought this was an issue linked to v pysam <12.

Acribbs commented 2 years ago

yeh I think I need to make a new release for cgat-apps. I can work on this now.

jscaber commented 2 years ago

Thanks! I tried to work on libcalignmentfile cimports there, but when changing it in peakshape.pyx, it failed and I didn’t understand why or the error message – will leave this to you.

From: Adam Cribbs @.> Reply to: cgat-developers/cgat-flow @.> Date: Wednesday, 24 November 2021 at 14:52 To: cgat-developers/cgat-flow @.> Cc: Jakub Scaber @.>, Mention @.***> Subject: Re: [cgat-developers/cgat-flow] cgat-flow-pipelines.yml causes installation of cgat-apps 0.6.0 (#143)

yeh I think I need to make a new release for cgat-apps. I can work on this now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/cgat-developers/cgat-flow/issues/143#issuecomment-977951744, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC4Y5MWAZUB3RV3GM6P3KT3UNT3ZVANCNFSM5C3YLQ7A. Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Acribbs commented 2 years ago

New version of cgat-apps was just merged to bioconda. Not sure how long it takes for it to be available though

jscaber commented 2 years ago

Availble now and has indeed fixed the issue: Commit 8a9a6a7 now passes on rerunning.

This would fix rnaseqqc which now works after @IanSudbery refactoring, but will further break windows, intervals (&other?) which are already defective. In the longer term the rpy2 pin will make the conda environment outdated and unresolvable, so my personal vote would be to unpin and move on.

Acribbs commented 2 years ago

I would unpin rpy2 and we can remove windows and intervals, dont know if anyone is using them anyway, im certainly not using them.