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

Obligatory metro map #96

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Obligatory metro map #96

wants to merge 4 commits into from
+1,176 −0

Conversation

edmundmiller
Copy link
Collaborator

Closes #68

@edmundmiller edmundmiller added this to the v2.1.0 milestone Nov 1, 2022
@edmundmiller edmundmiller self-assigned this Nov 1, 2022
@edmundmiller edmundmiller changed the title Obligatory subway map Obligatory metro map Nov 1, 2022
@github-actions
Copy link

github-actions bot commented Nov 1, 2022 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit b94ee9e

+| ✅ 154 tests passed       |+
#| ❔   3 tests were ignored |#
!| ❗   8 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in README.md: Add full-sized test dataset and amend the paragraph below if applicable
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release
  • pipeline_todos - TODO string in test_full.config: Specify the paths to your full test data ( on nf-core/test-datasets or directly in repositories, e.g. SRA)
  • pipeline_todos - TODO string in test_full.config: Give any required params for the test so that command line flags are not needed
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline
  • schema_description - No description provided in schema for parameter: assay_type
  • schema_description - No description provided in schema for parameter: skip_grohmm

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/email_template.html

✅ Tests passed:

Run details

  • nf-core/tools version 2.6
  • Run at 2022-11-01 04:38:13

@github-actions
Copy link

github-actions bot commented Nov 1, 2022

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 599d277

+| ✅ 154 tests passed       |+
#| ❔   3 tests were ignored |#
!| ❗   8 tests had warnings |!

❗ Test warnings:

  • pipeline_todos - TODO string in README.md: Add full-sized test dataset and amend the paragraph below if applicable
  • pipeline_todos - TODO string in WorkflowMain.groovy: Add Zenodo DOI for pipeline after first release
  • pipeline_todos - TODO string in test_full.config: Specify the paths to your full test data ( on nf-core/test-datasets or directly in repositories, e.g. SRA)
  • pipeline_todos - TODO string in test_full.config: Give any required params for the test so that command line flags are not needed
  • pipeline_todos - TODO string in awsfulltest.yml: You can customise AWS full pipeline tests as required
  • pipeline_todos - TODO string in methods_description_template.yml: #Update the HTML below to your prefered methods description, e.g. add publication citation for this pipeline
  • schema_description - No description provided in schema for parameter: assay_type
  • schema_description - No description provided in schema for parameter: skip_grohmm

❔ Tests ignored:

  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/email_template.html

✅ Tests passed:

Run details

  • nf-core/tools version 2.6
  • Run at 2022-11-01 04:35:12

jfy133
jfy133 reviewed Nov 1, 2022
View reviewed changes
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Some graphic design (personal) opinions:

  • Stick to a complementary colour theme, the Red is sort of OK but the turquoise blue looks 'off' to my eyes. I would recommend trying https://coolors.co/, fix the nf-core green and follow from that.
  • I don't understand the purpose of the red/green colours inuitiively either
  • Directionality is a bit difficult to follow for me, as PreSeq/BBmap/RSeQC look almost like insert points (as they are round), s I can't tell if they come off the main line or go inwards (due to the flowing 'into one' scheme into markduplicates. The filled in black dot FASTQ/multiqc bar isn't going to be inuititive to everything I think (Left/Right isn't default reading for many people - it's something I wasn't 💯 happ with the eager one, but I like with taxprofiler because I borrowed form the London bus network with the white arrows)
  • It's a a little cramped, particularly around the Markduplicates label and the transcript identification box, and also the nascent logo and preseq//bbmap etc. Feel free to use more white space as it's unlikely you'll be printing this regularly :)

@edmundmiller
Copy link
Collaborator Author

Thanks for the detailed feedback, and once again thanks for creating the templates and tutorials, they were amazing!

Stick to a complementary colour theme, the Red is sort of OK but the turquoise blue looks 'off' to my eyes. I would recommend trying https://coolors.co/, fix the nf-core green and follow from that.

The orange is actually the complementary color to the nf-core green and turquoise is part of our school colors (Which are actually green and orange). But thanks coolors looks awesome, I'll adjust those!

I don't understand the purpose of the red/green colours inuitiively either

That's what should go in the dead space in the top right-hand corner, a legend! 🙃

The turquoise was for GROSeq and the orange for everything else

I borrowed form the London bus network with the white arrows

That's great! I was trying to get at that they were subworkflows, but just metrics and graphs and didn't get used in the pipeline aftewards.

It's a a little cramped

I thought all of those spaces were a bit cramped as well, I think I took out a segment or two in each of them, I don't know why I was limiting myself 😬

@jfy133
Copy link
Member

jfy133 commented Nov 2, 2022

Re colours: fair enough! You don't have to stick with nf-core of course, just keep to a nice palette together :)

@edmundmiller edmundmiller removed this from the v2.1.0 milestone Jan 20, 2023
@sruthipsuresh
Copy link
Contributor

@jfy133 I've attached a copy of an edited map. We would really appreciate your feedback! There may be some font issues on my side but let me know if that looks good. Thank you for your time!
final_nascent

@jfy133
Copy link
Member

jfy133 commented Mar 28, 2023

Hi @sruthipsuresh

It's a bit hard for me to say overall as the SVG seems to be a bit borked when I cehck it (e.g. fonts).

But some feedback anyway:

  1. Alignment station is sqaure not circle
  2. The T-bars for end/step vs intermediate step is confusing, as I can't really see the difference
  3. Shouldn't the very beginning stop (before FastqC/Fastp) also be a filled circle?
  4. The lines going to BedTools genomecov/DeepTools BamCoverage are transparent, is that correct?
  5. Grouping of replicates doesn't have a station?
  6. I still find the lines coming out of Picard Markduplciates a bit confusing - are they inputs or outputs of the pipeline? I would try to angle 45 degrees with a bend to show which direction they are coming in/out from
  7. The bend finising on MultiQC is unnecessary I think and might give you a bit more space to shuffle things aroujdn

@sruthipsuresh
Copy link
Contributor

Hi @jfy133 , thank you so much for your detailed and prompt response! Here is a new iteration with your feedback incorporated (PDF format to prevent font issues).
I have also included two options to indicate input/outputs to mitigate the issue in 6:

  • Large arrowheads
  • Small arrowheads
    Let me know if that looks better! Thank you again for your time.
    final_nascent_edit_2.pdf

@jfy133
Copy link
Member

jfy133 commented Mar 29, 2023

Almost there arrows help a lot! The offset BEDTools intersect looks off, youc ould make the line/arrow leading to it shorter if you don't want to make the page bigger.

I still don't find the colours work well together personally, but I think I remeber @emiller88 says he is following some scheme somewhere, so I'll elave that ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jfy133 jfy133 jfy133 left review comments

@matthdsm matthdsm matthdsm approved these changes

Assignees

@edmundmiller edmundmiller

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create Subway pipeline
4 participants
@edmundmiller @jfy133 @sruthipsuresh @matthdsm