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: xengsort #2168

Merged
merged 9 commits into from Nov 17, 2023
Merged

New module: xengsort #2168

merged 9 commits into from Nov 17, 2023

Conversation

vladsavelyev
Copy link
Member

Fix #2165

Screenshot 2023-11-09 at 23 33 38

@vladsavelyev vladsavelyev added module: new awaits-review Awaiting final review and merge. labels Nov 9, 2023
@vladsavelyev vladsavelyev added this to the MultiQC v1.18 milestone Nov 9, 2023
Copy link
Contributor

github-actions bot commented Nov 9, 2023

🚀 Deployed on https://mqc-pr-2168--multiqc.netlify.app

@tamuanand
Copy link

tamuanand commented Nov 9, 2023

Hi @vladsavelyev

The help text for the actual xengsort classify command at https://mqc-pr-2168--multiqc.netlify.app/modules/xengsort/ should be changed - --method count should actually be --classification count such that it looks like

xengsort classify --index myindex \
  --fastq paired.1.fq.gz --pairs paired.2.fq.gz \
 --prefix myresults \
  --classification count \
 > sample.txt

I am guessing you probably grabbed that help text from their gitlab page

I have submitted an issue on their gitlab - https://gitlab.com/genomeinformatics/xengsort/-/issues/36#note_1638925835

I believe the docs here are incorrect - https://gitlab.com/genomeinformatics/xengsort#how-to-classify

it has it as --method count and I believe this should be --classification count based on CLI help using xengsort 2.0.1

[Jens Zentgraf](https://gitlab.com/jens.zentgraf)
[@jens.zentgraf](https://gitlab.com/jens.zentgraf)
Maintainer
Hi, thanks for pointing this out. I will update the README. Best Jens

@tamuanand tamuanand mentioned this pull request Nov 9, 2023
1 task
@tamuanand
Copy link

Great job @vladsavelyev

Some other suggestions:- Just like with xenome at #2164, for xengsort it would be great to have

  • a summary table in addition to the general stats column and hidden columns for read counts.
  • section wrappers to the plots to make sure they appear in the contents panel.

Thanks

@tamuanand
Copy link

I would suggest that xengsort summary stats table should be consistent with xenome summary stats table

  • hence, it would be better to have graft reads as column 1 (Human reads in xenome) and host reads as column 2 (Mouse reads in xenome)

@vladsavelyev vladsavelyev enabled auto-merge (squash) November 17, 2023 09:10
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.

LGTM!

Comment on lines +24 to +25

Then the sample name in the report will be `sample`, which is the base name of the file.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be the case.

Suggested change
Then the sample name in the report will be `sample`, which is the base name of the file.

@vladsavelyev vladsavelyev merged commit 1a8a91a into master Nov 17, 2023
7 checks passed
@ewels ewels deleted the xengsort branch November 17, 2023 10:25
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. module: new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Module request: xengsort
4 participants