-
Notifications
You must be signed in to change notification settings - Fork 13
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
spades: Update to 4. #216
spades: Update to 4. #216
Conversation
@@ -3,7 +3,7 @@ channels: | |||
- bioconda | |||
dependencies: | |||
- python <= 3.9 | |||
- spades = 3.15.4 | |||
- spades = 4.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it as simple as a drop in replacement like this? I would assume that the python version may need to be revisited as well, it may need to bump up to 3.11 or 3.12.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good q - is there a reason python is even specified? Shouldn't that just be handled as a dep of spades?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keeping it pinned below a certain version makes installation a lot more consistent for a longer time. Easier to pinpoint an issue when someone has an error
Suggested by @rhysnewell.
Pinned other stuff too - I guess that same logic applies. CI just broke, I think for unrelated reasons? I'm having trouble running tests locally due to HPC strangeness too. |
Hi @rhysnewell or @AroneyS this good to go now? |
Did the integration tests pass? |
test_short_read_assembly works for me |
Fixes #214.
Reported by: @michoug.
Suggested by: @AroneyS.