-
Notifications
You must be signed in to change notification settings - Fork 468
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
Add missing files for docker container building #54
base: master
Are you sure you want to change the base?
Conversation
This is a lot more than needed to run the tutorial, but maybe that's good so that people can use the env for further analysis as well. Have you tested that the whole workflow runs on this? |
Yes, it's indeed a lot more. Haven't run the tutorial with this in a while, but I can just start a testrun now 👍 |
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.
I like how everyone will have sfaira, episcanpy, and BED installed in their tutorial environment ;). Do you think it's worth it to separate mandatory from non-mandatory installs? Maybe just by comments in the dockerfile?
I don't mind, either way is fine for me. 🤷♂️ |
I don't mean commenting out packages, but just putting a comment in the Dockerfile saying "required install" and "optional extension" as headings? |
Also, I think that R package install will also take quite some time ^^ |
Btw... if you rerun the workflow, we could save that and update the "latest notebook" with an updated version of all dependencies! Possible you think? I would ofc go through results manually as well to check if it's more than just running through. |
Oh dear, looks like the scran installation in my current container fails 🙄
I'm tempted to just switch my containers to R 4.0 and Bioconductor 3.12 |
I have successfully avoided R 4.0 so far... this looks fairly painful. It might make sense to update, but then there will be more fun things coming up in the workflow, I imagine. |
Yes, should def do that... and remove some code that was just added in a PR to make the notebook compatible with latest tools. I would then tweet about it and @ you... off to the next 500 stars ;). |
Ok cool, I'll put building a R4 container on my list then. will wait until next week though as volker is releasing a new version of scVelo which annotates hvgs instead of filtering, want to have that in the container as well. Can you, as a first step, send me a cleaned version of the latest notebook that has the extra code removed and implements #20 and #39 ? I'll then give it a go and run it with the R4 container 🤞 |
I can do that... but maybe only on the weekend, as I have to finish these revisions and have to prep sth for a meeting on Friday as well. Although it shouldn't take too long... maybe I can fit in tomorrow night as well. |
No rush! As I said, I'll wait with the container building until the next scVelo release. So if you prep the notebook until some time next week that's fine :) |
Hi, @le-ander. I just met the exact same error as yours while installing
It seems the error is about the calculation of the nearest neighbor in the package May I ask how did you bypass the error? Thank you very much!!! My dockerfile is as follows:
|
FYI @le-ander @kuang-da I had this error come up in a different context, but what helped me was
It looks like something changed between 0.0.16 and 0.0.18 that brings up this error. |
Thanks! So some package is now using approximate nearest neighbour calculation that wasn't before I guess. |
also updated the dockerfile to the latest version we have.
closes #51