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

New module: PRINSEQ++ #1741

Merged
merged 12 commits into from Jan 7, 2023
Merged

New module: PRINSEQ++ #1741

merged 12 commits into from Jan 7, 2023

Conversation

jfy133
Copy link
Contributor

@jfy133 jfy133 commented Aug 10, 2022

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated
  • There is example tool output for tools in: Add test data for prinseqplusplus test-data#238
  • Code is tested and works locally (including with --lint flag)
  • docs/README.md is updated with link to below
  • docs/modulename.md is created
  • Everything that can be represented with a plot instead of a table is a plot
  • Report sections have a description and help text (with self.add_section)
  • There aren't any huge tables with > 6 columns (explain reasoning if so)
  • Each table column has a different colour scale to its neighbour, which relates to the data (eg. if high numbers are bad, they're red)
  • Module does not do any significant computational work

image

@jfy133 jfy133 changed the title Add module: PRINSEQ++ New module: PRINSEQ++ Aug 10, 2022
@ewels
Copy link
Member

ewels commented Jan 7, 2023

@jfy133 - code looks good, but two questions on the report output:

  1. The numbers in the General Stats table and the beeswarm look basically the same. Can we just have the 2-3 most important columns in GenStat and everything else in the plot?
  2. Can the beeswarm plot instead be a stacked bar plot? Do the numbers sum up to the correct total? I think that this would be far easier to read.

@jfy133
Copy link
Contributor Author

jfy133 commented Jan 7, 2023

Thanks @ewels!

  1. Not really - each filter is somewhat independent from one another (they aren't strictly additive, you could have one or the other depending on which flags you provide), and there is no default.
  2. They could theoretically be yes, I opted for the beeswarm as generally barcharts to me should represent fractions of the whole (in this case number of reads), whereas the log file just tells you the fractions that are removed - i.e., there is no 'input total' value to then represent the full 100% (if that makes sense). But I'm not that bothered as it's just a personal 'intutition', fine to have a barchart if that makes more sense to you.

@ewels
Copy link
Member

ewels commented Jan 7, 2023

Ok, let's sum the values for the general stats to avoid duplication and give the key figure.

No point doing the barplot if we don't have the read count total. Can add this in the future if that ever makes it into the log file.

@ewels ewels merged commit 23cb04c into MultiQC:master Jan 7, 2023
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.

None yet

2 participants