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.

Sign up for GitHub

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

Jump to bottom

Add taxonomy in workflow diagram #353

Closed
wants to merge 1 commit into from
Closed

Add taxonomy in workflow diagram #353

wants to merge 1 commit into from

Conversation

Darcy220606
Copy link
Contributor

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/funcscan branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 2, 2024 •
edited
Loading

nf-core lint overall result: Passed ✅

Posted for pipeline commit 0f951be

+| ✅ 227 tests passed       |+

✅ Tests passed:

Run details

  • nf-core/tools version 2.13.1
  • Run at 2024-04-02 11:27:15

@Darcy220606 Darcy220606 requested a review from a team April 2, 2024 14:40
jasmezz
jasmezz reviewed Apr 2, 2024
View reviewed changes
Copy link
Collaborator

@jasmezz jasmezz left a comment

Choose a reason for hiding this comment

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

  • For the annotation part (prokka etc.), the sequences actually come from gunzip, only tax. classification from MMseqs2. Atm. it looks like everything comes from tax. classification. It would be more correct to merge it like in my screenshot below.
  • I would call it "taxonomic classification".
  • I am not sure if it's better to write "MMseqs2" into the workflow and put "Taxonomic classification" into the legend (and give it a colour). That would be consistent to the 3 other subworkflows which have all their tools listed as stations. On the other hand, MMseqs2 is our only tool for the tax. class. and makes the diagram more clean like you put it. No strong opinion yet.
    image

@jasmezz
Copy link
Collaborator

jasmezz commented Apr 2, 2024

Maybe yours is actually better. Because the 3 coloured subworkflows also come from the tax. class. station, so maybe let the annotation seqs come out here as well to keep it consistent.

@Darcy220606
Copy link
Contributor Author

@jasmezz Hmm, I would suggest to have the three workflows coming out of the taxa classification 'stop' as that is how funcscan is now 'built' and yes I agree with u to rename the stop to mmseqs2 and colour the circle and add the coloured circle to the legend and add taxonomic classification there.

@jfy133
Copy link
Member

jfy133 commented Apr 4, 2024 •
edited
Loading

The current lines don't make sense to me, the mmseqs2 files don't go into the ARG/AMP/BGC workflows.

I would rather do something like this (please don't laugh at my 60second inkscape skills 😬 )

image

@Darcy220606
Copy link
Contributor Author

Darcy220606 commented Apr 4, 2024 •
edited
Loading

Nice drawing @jfy133 .... Yeah ok, thats actually closer to how it is setup. So i give it another colour and add another line in the legend for taxonomic classification. Back to the drawing board 🎨 👩🏽‍🎨

@jfy133
Copy link
Member

jfy133 commented Apr 4, 2024

Nice drawing @jfy133 .... Yeah ok, thats actually closer to how it is setup. So i give it another colour and add another line in the legend for taxonomic classification. Back to the drawing board 🎨 👩🏽‍🎨

Oooooor @Darcy220606 same black colour as annotation but different dashed line?

@jasmezz
Copy link
Collaborator

jasmezz commented Apr 17, 2024

Superseded by #361

@jasmezz jasmezz closed this Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasmezz jasmezz jasmezz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Darcy220606 @jasmezz @jfy133