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

add shigapass 1.5.0 #1041

Merged
merged 7 commits into from
Sep 5, 2024
Merged

add shigapass 1.5.0 #1041

merged 7 commits into from
Sep 5, 2024

Conversation

jvhagey
Copy link
Contributor

@jvhagey jvhagey commented Aug 28, 2024

This PR includes a Dockerfile for ShigaPass v1.5.0

Pull Request (PR) checklist:

  • Include a description of what is in this pull request in this message.
  • The dockerfile successfully builds to a test target for the user creating the PR. (i.e. docker build --tag samtools:1.15test --target test docker-builds/samtools/1.15 )
  • Directory structure as name of the tool in lower case with special characters removed with a subdirectory of the version number (i.e. spades/3.12.0/Dockerfile)
    • (optional) All test files are located in same directory as the Dockerfile (i.e. shigatyper/2.0.1/test.sh) -- they are in the container as they come with the software
  • Create a simple container-specific README.md in the same directory as the Dockerfile (i.e. spades/3.12.0/README.md)
    • If this README is longer than 30 lines, there is an explanation as to why more detail was needed
  • Dockerfile includes the recommended LABELS
  • Main README.md has been updated to include the tool and/or version of the dockerfile(s) in this PR
  • Program_Licenses.md contains the tool(s) used in this PR and has been updated for any missing

@kapsakcj
Copy link
Collaborator

Thanks for the PR. Could you please add a step in the app stage to index the database so that users do not need to index prior to running the tool? Perhaps add in another RUN command after line 44 in the dockerfile

I see you have the -u option used in the test stage but it would be helpful to have that done independently in the app stage

@jvhagey
Copy link
Contributor Author

jvhagey commented Aug 30, 2024

Need to change permissions on the database folder for use. I will update shortly.

@jvhagey
Copy link
Contributor Author

jvhagey commented Sep 3, 2024

@kapsakcj ok, you had a good suggestion so I incorporated that and provided details in the readme. Hopefully, its clear.

@kapsakcj
Copy link
Collaborator

kapsakcj commented Sep 5, 2024

Thanks for making those changes! Will save users from doing an important step. And thank you for documenting that in the README 👍

The tool-specific README looks great, thank you for putting that together. I made few small changes to this readme as well as the main /README.md table

I tested with a handful of S flexneri, S sonnei, and S dysenteriae genomes and ShigaPass ran without issue and correctly predicted serotypes.

My only complaint is the....semi-colon....delimited output format, but that's not within the scope here

@kapsakcj kapsakcj merged commit 7caff63 into StaPH-B:master Sep 5, 2024
2 checks passed
@kapsakcj
Copy link
Collaborator

kapsakcj commented Sep 5, 2024

Deploy workflow is here: https://github.com/StaPH-B/docker-builds/actions/runs/10712655108

The docker image should be available on dockerhub and quay.io shortly. Thank you!

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