search

MannLabs / alphatims

An open-source Python package for efficient accession and visualization of Bruker TimsTOF raw data from the Mann Labs at the Max Planck Institute of Biochemistry.
https://doi.org/10.1016/j.mcpro.2021.100149
Apache License 2.0
81 stars 26 forks source link

[Pitch] Docker Image #256

Closed jspaezp closed 1 year ago

jspaezp commented 1 year ago

Hello there! I was wondering if you would be interested in adding a docker image to the release cycle of the package.

I would gladly make PR for it if you want!

Thanks for the good work on the package :) -Sebastian

sander-willems-bruker commented 1 year ago

Hi @jspaezp , thanks for the kind words in interest. I have little to no experience with docker myself, but agree that it might be a valuable addition. Feel free to make a PR and we'll try and integrate it as well as possible. If it works out smoothly, I would be happy to collaborate actively and try to build some sort of template around it that might be useful for other packages in the AlphaPept ecosystem.

jspaezp commented 1 year ago

Hello @sander-willems-bruker ! that is great to hear, in my experience adding the docker file is fairly smooth for python packages (assuming one will use the GitHub container registry) ...

The docker file just uses the built distribution that would be pushed to pypi https://github.com/wfondrie/mokapot/blob/master/Dockerfile (example in a package that I am a co-maintainer in, that also uses conda as the main dependency and env manager)

And on the CI-CD side of things it just needs the addition of a gh action for it... Mokapot uses as separate workflow: https://github.com/wfondrie/mokapot/blob/master/.github/workflows/docker.yml

I have used my same deployment workflow (since I can just re-use the wheel build).

I think I could have a draft of the PR by Monday, LMK what branch you want me to use as a base for it :)

best, Sebastian

sander-willems-bruker commented 1 year ago

Sounds great! If you branch of development, we can merge it in there. Thanks again for the support.

jspaezp commented 1 year ago

Hello there! @sander-willems-bruker I have submitted a PR (#268), lmk if the scope is what you feel would be enough or if you have any additional requirements (more documentation/change CICD/permissions/additional things to bundle in the image....)

Best! Sebastian

sander-willems-bruker commented 1 year ago

Finally got to merging this in develop. Thanks for the help!

jspaezp commented 1 year ago

https://github.com/MannLabs/alphatims/pull/268#issuecomment-1648207397

posting here for context! Permissions required for a public image.

sander-willems-bruker commented 1 year ago

Yep, turns out this requires admin at organization level, I forwarded to the people that can enable this so hopefully will be integrated soon. If not, ping me;)

swillems commented 1 year ago

Dear @jspaezp . It took a little longer than expected, but the docker image should be available by now. Feel free to test it out and let me know what does and does not work!

jspaezp commented 1 year ago

Thanks a lot @swillems ! this is great!