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 sylph #816

Merged
merged 5 commits into from
Dec 8, 2023
Merged

Add sylph #816

merged 5 commits into from
Dec 8, 2023

Conversation

stephenturner
Copy link
Contributor

This PR adds sylph (https://github.com/bluenote-1577/sylph). sylph is a program that can perform ultrafast (1) ANI querying or (2) metagenomic profiling for metagenomic shotgun samples.

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)
  • Create a simple container-specific README.md in the same directory as the Dockerfile (i.e. spades/3.12.0/README.md)
  • 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

@stephenturner stephenturner changed the title add sylph 0.4.1 Dockerfile, sylph README, update main README Add sylph Dec 7, 2023
@kapsakcj
Copy link
Collaborator

kapsakcj commented Dec 7, 2023

Hi Dr. T, thanks for the PR! I'm excited to see you contribute here.

Myself or one of the other maintainers will review when we are able and make suggestions and/or request changes.

I just approved the CI workflow to run, so that will test that the image builds OK. Looks like it already passed successfully 👍

Upon a glance everything looks great, but when I get a chance I'll take a closer look.

Nice to see a rust base image in use here! I don't have a ton of experience with building rust packages so this will be a learning opportunity for me

sylph/0.4.1/README.md Outdated Show resolved Hide resolved
@erinyoung
Copy link
Contributor

I think this looks great!

I like the builder stage (which makes for a smaller image), and I like the test stage is using actual files.

I made two adjustments:

  1. I changed the hyperlink in the README to point to dockerhub (where the sylph image would be)
  2. I added in a sylph -h line to the test stage - just to make sure it's in PATH

@erinyoung
Copy link
Contributor

There is one warning during compiling. Is this something that can be ignored?

#10 50.29    Compiling sylph v0.4.1 (/sylph-0.4.1)
#10 50.84 warning: call to `.clone()` on a reference in this situation does nothing
#10 50.84    --> src/contain.rs:485:47
#10 50.84     |
#10 50.84 485 |         let file = File::open(read_sketch_file.clone()).expect(&format!(
#10 50.84     |                                               ^^^^^^^^ help: remove this redundant call
#10 50.84     |
#10 50.84     = note: the type `str` does not implement `Clone`, so calling `clone` on `&str` copies the reference, which does not do anything and can be removed
#10 50.84     = note: `#[warn(noop_method_call)]` on by default

@erinyoung
Copy link
Contributor

The results from the test look good

#16 [test 4/4] RUN date &&     sylph sketch -i e.coli-*.fasta -o database &&     sylph sketch o157_reads.fastq  &&     sylph query database.syldb *.sylsp > ani_queries.tsv &&     sylph profile database.syldb *.sylsp > profiling.tsv &&     cat *.tsv &&     date
#16 0.053 Thu Dec  7 20:14:23 UTC 2023
#16 0.055 2023-12-07T20:14:23.855Z INFO  [sylph::sketch] Sketching genomes...
#16 0.087 2023-12-07T20:14:23.887Z INFO  [sylph::sketch] Wrote all genome sketches to database.syldb
#16 0.088 2023-12-07T20:14:23.888Z INFO  [sylph::sketch] Finished.
#16 0.091 2023-12-07T20:14:23.891Z INFO  [sylph::sketch] Sketching non-paired sequences...
#16 0.104 2023-12-07T20:14:23.904Z INFO  [sylph::sketch] Sketching ./o157_reads.fastq.sylsp complete.
#16 0.104 2023-12-07T20:14:23.905Z INFO  [sylph::sketch] Finished.
#16 0.106 2023-12-07T20:14:23.906Z INFO  [sylph::contain] Obtaining sketches...
#16 0.108 2023-12-07T20:14:23.908Z INFO  [sylph::contain] Finished obtaining genome sketches.
#16 0.122 2023-12-07T20:14:23.922Z INFO  [sylph::contain] Finished sample o157_reads.fastq.sylsp.
#16 0.122 2023-12-07T20:14:23.922Z INFO  [sylph::contain] sylph finished.
#16 0.124 2023-12-07T20:14:23.924Z INFO  [sylph::contain] Obtaining sketches...
#16 0.125 2023-12-07T20:14:23.926Z INFO  [sylph::contain] Finished obtaining genome sketches.
#16 0.143 2023-12-07T20:14:23.943Z INFO  [sylph::contain] o157_reads.fastq.sylsp taxonomic profiling; reassigning k-mers for 4 genomes...
#16 0.171 2023-12-07T20:14:23.971Z INFO  [sylph::contain] o157_reads.fastq.sylsp has 2 genomes passing profiling threshold. 
#16 0.171 2023-12-07T20:14:23.971Z INFO  [sylph::contain] Finished sample o157_reads.fastq.sylsp.
#16 0.171 2023-12-07T20:14:23.971Z INFO  [sylph::contain] sylph finished.
#16 0.173 Sample_file	Genome_file	Adjusted_ANI	Eff_cov	ANI_5-95_percentile	Eff_lambda	Lambda_5-95_percentile	Median_cov	Mean_cov_geq1	Containment_ind	Naive_ANI	Contig_name
#16 0.173 o157_reads.fastq	e.coli-o157.fasta	100.00	0.200	98.66-102.43	0.200	0.10-0.32	1	1.740	77/366	95.10	NZ_CP017439.1 Escherichia coli O157:H7 strain 2159 plasmid pO157, complete sequence
#16 0.173 o157_reads.fastq	e.coli-o157.fasta	99.74	0.361	99.61-99.94	0.361	0.34-0.38	1	1.188	6149/21556	96.03	NZ_CP017438.1 Escherichia coli O157:H7 strain 2159 chromosome, complete genome
#16 0.173 o157_reads.fastq	e.coli-EC590.fasta	98.25	0.319	98.06-98.55	0.319	0.29-0.34	1	1.172	3122/19330	94.29	NZ_CP016182.2 Escherichia coli strain EC590 chromosome, complete genome
#16 0.173 o157_reads.fastq	e.coli-K12.fasta	98.16	0.327	97.96-98.47	0.327	0.29-0.35	1	1.171	3114/19485	94.26	NC_007779.1 Escherichia coli str. K-12 substr. W3110, complete sequence
#16 0.173 Sample_file	Genome_file	Taxonomic_abundance	Sequence_abundance	Adjusted_ANI	Eff_cov	ANI_5-95_percentile	Eff_lambda	Lambda_5-95_percentile	Median_cov	Mean_cov_geq1	Containment_ind	Naive_ANI	Contig_name
#16 0.173 o157_reads.fastq	e.coli-o157.fasta	64.3699	99.0626	99.74	0.361	99.61-99.94	0.361	0.34-0.38	1	1.188	6147/21556	96.03	NZ_CP017438.1 Escherichia coli O157:H7 strain 2159 chromosome, complete genome
#16 0.173 o157_reads.fastq	e.coli-o157.fasta	35.6301	0.9374	100.00	0.200	98.66-102.43	0.200	0.10-0.32	1	1.740	77/366	95.10	NZ_CP017439.1 Escherichia coli O157:H7 strain 2159 plasmid pO157, complete sequence

@erinyoung
Copy link
Contributor

I think we can merge this PR, but I'd like clarity that the compilation warning can be ignored.

@stephenturner
Copy link
Contributor Author

There is one warning during compiling. Is this something that can be ignored?

#10 50.29    Compiling sylph v0.4.1 (/sylph-0.4.1)
#10 50.84 warning: call to `.clone()` on a reference in this situation does nothing
#10 50.84    --> src/contain.rs:485:47
#10 50.84     |
#10 50.84 485 |         let file = File::open(read_sketch_file.clone()).expect(&format!(
#10 50.84     |                                               ^^^^^^^^ help: remove this redundant call
#10 50.84     |
#10 50.84     = note: the type `str` does not implement `Clone`, so calling `clone` on `&str` copies the reference, which does not do anything and can be removed
#10 50.84     = note: `#[warn(noop_method_call)]` on by default

I wish I knew the answer to this. I think the rust compiler is telling you that this .clone() isn't necessary.

@erinyoung
Copy link
Contributor

It's probably fine (seems to run okay).

I'm going to

  1. merge this PR
  2. deploy sylph to dockerhub and quay with the tags "0.4.1" and "latest"

@erinyoung erinyoung merged commit f810cb7 into StaPH-B:master Dec 8, 2023
2 checks passed
@erinyoung
Copy link
Contributor

Thank you for your contribution!

You can check the status of the deploy here https://github.com/StaPH-B/docker-builds/actions/runs/7135566344

Let us know if you run into problems

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.

3 participants