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

Sentieon #1239

Merged
merged 36 commits into from
Nov 11, 2020
Merged

Sentieon #1239

merged 36 commits into from
Nov 11, 2020

Conversation

chrispyatt
Copy link
Contributor

@chrispyatt chrispyatt commented Jun 29, 2020

NA12878.AlignmentStat_metrics.txt
NA12878.GCBias_metrics.txt
NA12878.InsertSize_metrics.txt

  • This comment contains a description of changes (with reason) - New Sentieon module added to catch output from Sentieon DNASEQ tools (the output has a modified header which is not caught by the picard module even with appropriate config).
  • CHANGELOG.md has been updated
  • There is example tool output for tools in the https://github.com/ewels/MultiQC_TestData repository or attached to this PR
  • Code is tested and works locally (including with --lint flag) -- throws warning about sentieon being found in installed modules but not README.md, but it definitely is in README.md so I can't work out the source of this error.
  • 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

@chrispyatt chrispyatt marked this pull request as ready for review June 30, 2020 08:31
ewels added a commit to MultiQC/test-data that referenced this pull request Nov 11, 2020
@ewels ewels merged commit 7492ed7 into MultiQC:master Nov 11, 2020
@ewels
Copy link
Member

ewels commented Nov 11, 2020

Many thanks for this! I added a couple of minor tweaks and fixed the merge conflict. But generally I thought that the code looked great and seems to work well 👍🏻

Sorry for the delay in reviewing & merging, thanks for the new module! 🚀

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

3 participants