-
Notifications
You must be signed in to change notification settings - Fork 250
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
Update Cellpose from 2.2.2 to 3.0.1 #559
Conversation
No test-cmds.txt (test file) present, skipping tests |
This PR adds the newest version of Cellpose, some of the changes:
Currently, there is a chmod 777 command for permissions to folder |
Hello! Thanks for the PR. Not sure what do you mean regarding writing permissions. At which step are the permissions required? |
Thanks for the comments @mboudet! I was referring to this line: https://github.com/FloWuenne/containers/blob/12a2df77f99ec7e77cd6edb8fa0399f0d209d93a/cellpose/2.3.2/Dockerfile#L28 We were having issues with permissions when running cellpose from within the container when not making the ./cellpose directory and giving it 777 permissions. Generally its quite bad practice to use |
@mboudet I have merged the pip commands into one Layer and have changed the permission by adding a user, giving that user permissions and running the container as that user. This was suggested by ChatGPT but I have seen this approach suggested in some forums too. Is this an acceptable approach? |
No test-cmds.txt (test file) present, skipping tests |
No test-cmds.txt (test file) present, skipping tests |
Thanks for the changes regarding pip! I'm guessing the permission issue is for Singularity? (Given that the docker is run as root, it should not matter for a |
@mboudet Actually the permission issue is not for singularity but is affecting docker when using the container with Nextflow. I was unable to make the container run in Nextflow with any setting beside forcing the user to be root in Nextflow. I don't quite understand why the permissions within nextflow need to be root for this specific container and the folders it creates when downloading models... I will check out the new release 3.0.0 and if that fixes these issues we might just skip this one and go directly for 3.0.0! |
Oh, interesting. I don't usually run nextflow with docker containers (only singularity), so I'm not sure what is happening permission-wise. In any case, I don't think the chmod 777 is an issue. You have it set to 755 currently though, so it will only work if it needs read permission (and not write). Alright for the release 3.0.0, you can juste update this PR with the correct release when it's ready. |
@mboudet I have updated to version 3.0.1 after testing that it works with old trained models and within our nextflow setup. I have also figured out the issue with permission. We needed to add a new variable: This moves the models that are downloaded out of $HOME and thus removes the permissions issues. This image should now be good for version 3.0.1! |
No test-cmds.txt (test file) present, skipping tests |
Perfect, thank you! |
Submitting a Container
(If you're requesting for a new container, please check the procedure described here.
Check BioContainers' Dockerfile specifications
Checklist
Check BioContainers' Dockerfile metadata