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

omit --dirs from the MultiQC invocation to only show sample names in report #467

Merged
merged 6 commits into from
Apr 28, 2023

Conversation

tomkinsc
Copy link
Member

omit --dirs from the MultiQC invocation to only show sample names in report; and provide a boolean to optionally enable display of full paths

Addresses #466

…report

omit --dirs from the MultiQC invocation to only show sample names in report; and provide a boolean to optionally enable display of full paths
@@ -464,6 +464,7 @@ task MultiQC {
Array[String]? module_to_use
Boolean data_dir = false
Boolean no_data_dir = false
Boolean show_sample_names_with_paths = false
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why we’d actually want to support the —dirs argument in WDL? We could probably drop this parameter

Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically to disambiguate same-name samples

Copy link
Member

Choose a reason for hiding this comment

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

If the "sample name" (without the --dirs argument) is simply the basename of the filename, I think we already did some work earlier to ensure that our demux task was emitting unique filenames for everything (e.g. appending lane numbers and such to the filename so that our nextseq output files were all uniquely named)

@@ -447,7 +447,7 @@ task aggregate_metagenomics_reports {

task MultiQC {
input {
Array[File] input_files = []
Array[File] input_files
Copy link
Member

Choose a reason for hiding this comment

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

I think the original thinking for this default empty assignment was so execution engines like Terra and dnanexus wouldn’t bother the user by default to fill it in if they wanted to omit it. That said… I can’t think off the top of my head where we’d currently have the multiqc task called in a workflow with inbound inputs so maybe this was pointless

@tomkinsc tomkinsc merged commit 93e293a into master Apr 28, 2023
@tomkinsc tomkinsc deleted the ct-multiqc-report-samplename-display-fix branch April 28, 2023 20:36
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.

2 participants