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: sourmash compare #1812

Merged
merged 27 commits into from Sep 14, 2023
Merged

New module: sourmash compare #1812

merged 27 commits into from Sep 14, 2023

Conversation

taylorreiter
Copy link
Contributor

This PR adds a new multiqc module for the output of sourmash compare (#1805). It was modeled after the vcftools relatedness2 module. I left space for future sourmash modules that I will implement soon.

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated

New module checklist:

  • 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)
  • 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

Test data:
This test data is two files, the *labels.txt file and the numpy array that is output alongside the labels.txt document. I've zipped them together to allow for upload to the PR, but they should be unzipped and uploaded as separate files to the https://github.com/ewels/MultiQC_TestData repository.
gut_compare.zip

@taylorreiter taylorreiter mentioned this pull request Dec 9, 2022
11 tasks
CHANGELOG.md Outdated Show resolved Hide resolved
@vladsavelyev vladsavelyev self-requested a review September 7, 2023 11:45
Copy link
Member

@vladsavelyev vladsavelyev left a comment

Choose a reason for hiding this comment

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

Thank you for writing this module!

I made some refactoring, mainly:

  • the extra compare2matrix felt like an unnecessary abstraction
  • simplified parsing - data.tolist() is extactly the input heatmap.plot expects, so no need to go through an extra intermediate dict (and in the future, we should even be able to use numpy arrays directly without extra copying!)
  • minor clean up and uncommenting self.write_data_file()

@vladsavelyev vladsavelyev added the awaits-review Awaiting final review and merge. label Sep 7, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
ewels added a commit to MultiQC/test-data that referenced this pull request Sep 13, 2023
Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Couple of very small comments, but generally looks great and happy for this to be merged once those are addressed 👍🏻

CHANGELOG.md Outdated Show resolved Hide resolved
multiqc/modules/sourmash/compare.py Outdated Show resolved Hide resolved
multiqc/modules/sourmash/compare.py Outdated Show resolved Hide resolved
@ewels ewels merged commit e3907d0 into MultiQC:master Sep 14, 2023
11 checks passed
@taylorreiter taylorreiter deleted the ter/add-sourmash-compare branch October 5, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits-review Awaiting final review and merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants