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

Seeker integration #109

Merged
merged 15 commits into from
Oct 23, 2020
Merged

Seeker integration #109

merged 15 commits into from
Oct 23, 2020

Conversation

papanikos
Copy link
Contributor

Hi team,

I saw seeker was on your wishlist #1 #75 , it came up on my and figured I 'd give it a go.

A short summary of this PR:

  • Dockerfile for seeker
    • I am using a conda env within docker. I was testing this and it seems to work
    • I am not sure how you want to handle containers. It would be nice to have them under one "organization". For now I put in my own Docker hub. Feel free to grab it, rewrite it, whatever.
  • Seeker run module, filtering based on author's 0.5 threshold, seeker_wf within main phage.nf
  • Inclusion in README, minor stylistic changes in there.
  • Addition of reference in the the Citations.txt
  • Removal of Docs folder, in favor of docs (maybe a typo in some PR?)

Profiles tested:

  • -profile smalltest,local,docker
    • --anno, turn off all tools except deepvirfinder and seeker
  • -profile smalltest,local,singularity
    • --anno, turn off all tools except deepvirfinder and seeker
  • -profile smalltest,slurm,singularity (our hpc doesn't support docker)
    • full run with all tools, identification and annotation on

All seem to run as expected for me.

Notes

  • I haven't really tested performance of seeker on a big dataset. It provides a minimal command line interface without any options for controlling runtime parameters (e.g. cpu usage). Maybe a wrapper as described here included as
run_seeker.py -i <fasta> -o <output.tsv> -c <cores

in the container would add some flexibility. With that said, in the containers config I used parameters similar to other tools that seemed sensible.

Glad to pick any issue during review.

Cheers,
Nikos

@replikation replikation added the enhancement New feature or request label Oct 21, 2020
@replikation
Copy link
Owner

  • thank you very much for this :) , we will review all the steps, test on big datasets and implement this - that's quite helpful
  • you installed docker via conda? can you share the code for this, we currently are writing a "read the docs" and could add this part for conda users :)
  • good catch on the --anno part we might have a bug here on our end

@papanikos
Copy link
Contributor Author

papanikos commented Oct 21, 2020

Glad to contribute!

What I meant with "conda env within docker", should be "install conda inside docker (available from the base image anyway) and create an environment inside the docker container"

Some references

FWIW, I didn't bother with testing if there is any working package available through conda. I had to go through setting up docker on my own Ubuntu 18 laptop.

Singularity and nextflow work though.

$ conda create -n wtp nextflow==20.07.01 singularity==3.6
$ conda activate wtp
(wtp) $ nextflow run replikation/What_the_Phage -r 0.9.0 --setup ...

is my current setup, and it works with -profile local,singularity.

@replikation
Copy link
Owner

  • ah yes i understand now. yep conda install on singularity works, there might be some issues on a cluster environment however compared to a native install of singularity
    thanks

@replikation replikation self-requested a review October 22, 2020 13:47
Copy link
Owner

@replikation replikation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mult1fractal mult1fractal merged commit 1e2be4e into replikation:master Oct 23, 2020
@papanikos papanikos deleted the seeker branch October 28, 2020 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Priority MEDIUM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants