Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] usesourmash prefetch from sourmash v4.1.0 #68

Merged
merged 18 commits into from
May 18, 2021
Merged

Conversation

ctb
Copy link
Member

@ctb ctb commented Apr 24, 2021

Update genome-grist to use the sourmash prefetch code from sourmash-bio/sourmash#1370, instead of its own implementation.

This also updates #76 so that the temporary directory is configurable in the conf files; briefly, you can now specify a list of possible temp directory paths, and they will be tried one at a time. Also removes a possible collision problem where if two people are working on the same sample on the same system, the first one "wins" the tempdir name; tempdirs no longer collide.

🎉 @bluegenes

@bluegenes
Copy link
Member

First pass testing -- prefetch with a ZipFile index seems to have worked!

@ctb ctb changed the title [WIP] switch over to using alpha-versionsourmash prefetch code [MRG] usesourmash prefetch from sourmash v4.1.0 May 15, 2021
@ctb
Copy link
Member Author

ctb commented May 15, 2021

Ready for review and merge, @bluegenes!

@ctb
Copy link
Member Author

ctb commented May 18, 2021

ping @taylorreiter @bluegenes for a quick review :)

Copy link
Member

@bluegenes bluegenes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay 4.1!

I really like the base_tempdir change -- was a bit worried about potential drawbacks of just setting /scratch

@ctb
Copy link
Member Author

ctb commented May 18, 2021

yay!

@ctb ctb merged commit 8b9f26d into latest May 18, 2021
@ctb ctb deleted the add/prefetch_cli branch May 18, 2021 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants