rotary-genomics / rotary

Assembly/annotation workflow for Nanopore-based microbial genome data containing circular DNA elements
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Separate utilities into spokewrench #152

Closed jmtsuji closed 2 months ago

jmtsuji commented 2 months ago

This PR removes the circularization-related utilities from rotary, given that these have been ported to rotary-utils spokewrench. The README is updated to reflect these changes.

jmtsuji commented 2 months ago

Contingent on rotary-utils spokewrench PR number 1.

End-to-end installation and run testing hasn't yet been performed on this draft PR.

LeeBergstrand commented 2 months ago

@jmtsuji Do we want to address https://github.com/rotary-genomics/rotary/issues/111 in this branch? I think with your code moved, we can move to Snakemake 8.0 pretty easily. This will require changes to Pungi as well.

LeeBergstrand commented 2 months ago

@jmtsuji We might simplify enviroment.yml, meta.yml, and pyproject.toml once the rotary-utils has an independent installer. However, we might have to wait until rotary-utils are in conda before installing it via snakemake.

LeeBergstrand commented 2 months ago

@jmtsuji We might be able to add to the install instructions for snakemake to use a pre-baked environment with rotary-utils installed.

https://stackoverflow.com/questions/59107413/activating-existing-conda-enviornments-in-snakemake

So you would tell the user in the readme to create a specific named environment with rotary-utils installed.

jmtsuji commented 2 months ago

@jmtsuji We might be able to add to the install instructions for snakemake to use a pre-baked environment with rotary-utils installed.

https://stackoverflow.com/questions/59107413/activating-existing-conda-enviornments-in-snakemake

So you would tell the user in the readme to create a specific named environment with rotary-utils installed.

@LeeBergstrand Good thoughts about #111. OK, let's try the "pre-baked environment" approach you've suggested as a temporary way to move rotary-utils into a separate env from rotary. In the rotary README, we can add install instructions for the user to make a conda env with a specific env name that includes rotary-utils, and that conda env can be called in rules where rotary-utils is needed. I'll take a look at this next week, and if all goes smoothly, I'll aim to integrate it into this PR.

We should then be able to simplify the dependencies in the install files for rotary, like you've mentioned. Assuming that the "pre-baked environment" solution works, I should be able to simplify the install files as part of this PR.

For Snakemake, I wonder if it would be better to wait to upgrade to version 8 until a subsequent PR, after rotary-utils is separated from rotary. The Snakemake 8 upgrade might be straightforward, but on the other hand, it might also cause other unexpected consequences that would be better to troubleshoot independently from trying to separate rotary-utils. These are just my initial thoughts... what do you think? Thanks!

LeeBergstrand commented 2 months ago

user to make a conda env with a specific env name that includes rotary-utils

We might be able to add the name of the environment to use to the config as well.

For Snakemake, I wonder if it would be better to wait to upgrade to version 8 until a subsequent PR, after rotary-utils is separated from rotary. The Snakemake 8 upgrade might be straightforward, but on the other hand, it might also cause other unexpected consequences that would be better to troubleshoot independently from trying to separate rotary-utils. These are just my initial thoughts... what do you think? Thanks!

@jmtsuji Let's move to version 8 in a separate PR, as you suggested.

jmtsuji commented 2 months ago

We might be able to add the name of the environment to use to the config as well.

I can test out if this is possible -- will be a nice feature.

Let's move to version 8 in a separate PR, as you suggested.

OK, sounds good.

I'll add these additional updates to this PR and will let you know once the PR is ready for review.

jmtsuji commented 2 months ago

@LeeBergstrand This PR is now updated to reflect use of spokewrench. Next week, I'll work on implementing the pre-baked conda env discussed above. I'll then send you a review request.

jmtsuji commented 2 months ago

@LeeBergstrand Ready for review. Like we discussed, rather than using a pre-baked conda environment, I moved the installs of spokewrench and pungi into conda environment files so they are installed automatically. This PR should simplify installation of rotary and allow us to move to snakemake 8 in future. Look forward to your review!