search

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

Major rotary upgrades #94

Closed jmtsuji closed 8 months ago

jmtsuji commented 9 months ago

Rotary now includes short read QC, a Python-based end repair script, updated databases (e.g., GTDB), multi-sample run support, and a basic CLI.

jmtsuji commented 9 months ago

We should run an end-to-end test with the current version to make sure everything is working.

jmtsuji commented 8 months ago

End-to-end test basically passes. One contig failed the end repair step, but I think this could be due to a stochastic difference of how the contig was rotated when it was assembled. The end repair step is a bit finicky right now -- I hope to clean this up when I finish the internal stitch module in rotary. For now, would it make sense to change the rotary config defaults to let un-repaired circular contigs pass through the end repair module without error? i.e., change keep_unrepaired_contigs: 'False' to keep_unrepaired_contigs: 'True'?

jmtsuji commented 8 months ago

@LeeBergstrand We can use the misc_changes branch for miscellaneous improvements during this final pre-merge code review. I added the rotary logo draft to the README in that branch.

LeeBergstrand commented 8 months ago

Ya you can leave the dev into master branch open and then make a misc branch off develop and then merge that branch into dev without closing the dev to master pull request.

On Dec 12, 2023, at 10:14 PM, Jackson M. Tsuji @.***> wrote:

@LeeBergstrand https://github.com/LeeBergstrand We can use the misc_changes https://github.com/rotary-genomics/rotary/tree/misc_changes branch for miscellaneous improvements during this final pre-merge code review. I added the rotary logo draft to the README in that branch.

— Reply to this email directly, view it on GitHub https://github.com/rotary-genomics/rotary/pull/94#issuecomment-1853328217, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMMYRUFYI4KVP4IVVFU3T3YJFBSZAVCNFSM6AAAAABAQX5TQSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJTGMZDQMRRG4. You are receiving this because you were mentioned.

jmtsuji commented 8 months ago

Ya you can leave the dev into master branch open and then make a misc branch off develop and then merge that branch into dev without closing the dev to master pull request.

Agreed -- sounds good! I will leave the dev branch open and the PR from dev to master open. We can make changes in the misc branch during code review of dev. Then, once done with the code review, we can make a PR from misc into dev, then resolve the PR from dev into master. Let me know if I didn't understand anything correctly -- thanks.

LeeBergstrand commented 8 months ago

End-to-end test basically passes. One contig failed the end repair step, but I think this could be due to a stochastic difference of how the contig was rotated when it was assembled. The end repair step is a bit finicky right now -- I hope to clean this up when I finish the internal stitch module in rotary. For now, would it make sense to change the rotary config defaults to let un-repaired circular contigs pass through the end repair module without error? i.e., change keep_unrepaired_contigs: 'False' to keep_unrepaired_contigs: 'True'?

Agreed, this was one of the changes I was thinking about suggesting.

LeeBergstrand commented 8 months ago

@jmtsuji We could probably merge this in and address issues after the fact. I would leave the version numbers alone because changing them to -beta may break the CLI installer. Parsing of pyproject.toml files and using them for installation has very specific requirements. For example, it wouldn't take the naming scheme for the dependencies, and I had to rewrite the dependency versions to specific specs.

LeeBergstrand commented 8 months ago

@jmtsuji We could probably merge this in and address issues after the fact. I would leave the version numbers alone because changing them to -beta may break the CLI installer. Parsing of pyproject.toml files and using them for installation has very specific requirements. For example, it wouldn't take the naming scheme for the dependencies, and I had to rewrite the dependency versions to specific specs.

I added the -beta1 to the version name and tested it. It worked.

jmtsuji commented 8 months ago

@LeeBergstrand Thanks for your quick input! I've reviewed your edits in the misc_changes branch and have merged them in. I've also created a PR with my demo code added to the README. If everything looks OK to you, would you mind merging in the demo branch into develop and then merge develop into master? Thanks!

LeeBergstrand commented 8 months ago

@LeeBergstrand Thanks for your quick input! I've reviewed your edits in the misc_changes branch and have merged them in. I've also created a PR with my demo code added to the README. If everything looks OK to you, would you mind merging in the demo branch into develop and then merge develop into master? Thanks!

@jmtsuji I changed the polca fix branch so the entire polca directory is temped. I decided to merge it all in because of time requirements. I couldn't do a complete end-to-end test of this branch because our work server is down right now (networking issue), but I'll test it soon when we are back up and do a hotfix to master if necessary.