Retain read group info by TCLamnidis · Pull Request #809 · nf-core/eager

TCLamnidis · 2021-11-18T15:20:40Z

Changes:

When mapping, assign the SM field of read groups to the sample name instead of the library Id.
Deactivate replacement of read headers upon library merging.
Flexible filtering for map and base quality when genotyping with pileupcaller.

This combination (in my small testing case) skips all the fiddling with read groups manually, retains all read group information and still produces a single genotyped individual per BAM within gatk UG.

Still testing a couple of things, but this should retain all the library/sequencing information within the final bams produced.

Added Parameters:

pileupcaller_min_map_quality. Default is 30. Changes the minimum mapping quality used in mpileup creation for pileupcaller. Affects samtools mpileup -q parameter.
pileupcaller_min_base_quality. Default is 30. Changes the minimum base quality used in mpileup creation for pileupcaller. Affects samtools mpileup -Q parameter.

PR checklist

This comment contains a description of changes (with reason).
Make sure your code lints (nf-core lint .).
Ensure the test suite passes (nextflow run . -profile test,docker).
Usage Documentation in docs/usage.md is updated.
Output Documentation in docs/output.md is updated.
CHANGELOG.md is updated.
README.md is updated (including new tool citations and authors/contributors).

github-actions · 2021-11-18T15:21:01Z

This PR is against the `master` branch ❌

Do not close this PR
Click Edit and change the base to dev
This CI test will remain failed until you push a new commit

Hi @TCLamnidis,

It looks like this pull-request is has been made against the nf-core/eager master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the nf-core/eager dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

github-actions · 2021-11-18T15:21:09Z

Markdown linting is failing

To keep the code consistent with lots of contributors, we run automated code consistency checks.
To fix this CI test, please run:

Install markdownlint-cli
- On Mac: brew install markdownlint-cli
- Everything else: Install npm then install markdownlint-cli (npm install -g markdownlint-cli)
Fix the markdown errors
- Automatically: markdownlint . --config .github/markdownlint.yml --fix
- Manually resolve anything left from markdownlint . --config .github/markdownlint.yml

Once you push these changes the test should pass, and you can hide this comment 👍

We highly recommend setting up markdownlint in your code editor so that this formatting is done automatically on save. Ask about it on Slack for help!

Thanks again for your contribution!

…eupcaller_params

Flexible filtering of reads for pileupcaller genotyping.

TCLamnidis · 2021-11-22T12:30:02Z

                    "type": "number",
                    "description": "Minimum percent identity alignments are required to have to be reported. Recommended to set same as MALT parameter.",
-                    "default": 85,
+                    "default": 85.0,


Suggested change

"default": 85.0,

"default": 85,

jfy133

Mostly cleanup things but otherwise LGTM

jfy133 · 2021-11-23T19:25:46Z

    ## Have to set validation as lenient because of BWA issue: "I see a read stands out the end of a chromosome and is flagged as unmapped (flag 0x4). [...]" http://bio-bwa.sourceforge.net/
-    picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT
-    samtools index ${libraryid}_seqtypemerged_rg.bam ${size}
+    # picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT


Suggested change

# picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT

# picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT

Suggested change

# picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT

# picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT

I would remove the entire line, not leave it commented

You can also remove the comment as well if necessary

jfy133 · 2021-11-23T19:26:52Z

  ## Have to set validation as lenient because of BWA issue: "I see a read stands out the end of a chromosome and is flagged as unmapped (flag 0x4). [...]" http://bio-bwa.sourceforge.net/
-  picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${samplename}_libmerged_rmdup.bam O=${samplename}_libmerged_rg_rmdup.bam RGID=1 RGLB="${samplename}_merged" RGPL=illumina RGPU=4410 RGSM="${samplename}_merged" VALIDATION_STRINGENCY=LENIENT
-  samtools index ${samplename}_libmerged_rg_rmdup.bam ${size}
+  # picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${samplename}_libmerged_rmdup.bam O=${samplename}_libmerged_rg_rmdup.bam RGID=1 RGLB="${samplename}_merged" RGPL=illumina RGPU=4410 RGSM="${samplename}_merged" VALIDATION_STRINGENCY=LENIENT


Comment above applies to all lib merges

Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>

Sample name as SM in bam read groups. Retain RG info on bam merges

21a9b1f

TCLamnidis added the WIP Work in progress label Nov 18, 2021

TCLamnidis changed the base branch from master to dev November 18, 2021 15:21

TCLamnidis self-assigned this Nov 18, 2021

TCLamnidis linked an issue Nov 18, 2021 that may be closed by this pull request

Retain read group information in bam merging steps #808

Closed

TCLamnidis added 3 commits November 18, 2021 16:32

Update CHANGELOG.md

ef525d1

Flexible filtering of reads for pileupcaller genotyping.

79fedba

Fix typo

0344f01

TCLamnidis commented Nov 18, 2021

View reviewed changes

Comment thread main.nf Outdated

TCLamnidis commented Nov 18, 2021

View reviewed changes

Comment thread main.nf Outdated

TCLamnidis commented Nov 18, 2021

View reviewed changes

Comment thread main.nf Outdated

TCLamnidis added 5 commits November 18, 2021 19:04

Correct bam merging output name

6cf64b7

Correct process output for bam merging proceses

5bbcd30

Merge remote-tracking branch 'origin/retain_read_group_info' into pil…

b0232d9

…eupcaller_params

Update CHANGELOG.md

2db00fb

Merge pull request #810 from nf-core/pileupcaller_params

74e79ba

Flexible filtering of reads for pileupcaller genotyping.

TCLamnidis commented Nov 22, 2021

View reviewed changes

TCLamnidis requested a review from jfy133 November 23, 2021 15:03

Update CHANGELOG.md

b5802f9

jfy133 requested changes Nov 23, 2021

View reviewed changes

TCLamnidis and others added 2 commits November 23, 2021 21:19

Update CHANGELOG.md

fb124c2

Co-authored-by: James A. Fellows Yates <jfy133@gmail.com>

Removed commented old code.

cecbf0b

TCLamnidis removed the WIP Work in progress label Nov 24, 2021

Merge branch 'dev' into retain_read_group_info

4228c42

jfy133 merged commit b567e33 into dev Nov 24, 2021

TCLamnidis deleted the retain_read_group_info branch January 12, 2022 15:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Retain read group info#809

Retain read group info#809
jfy133 merged 13 commits intodevfrom
retain_read_group_info

TCLamnidis commented Nov 18, 2021 •

edited

Loading

Uh oh!

github-actions Bot commented Nov 18, 2021

Uh oh!

github-actions Bot commented Nov 18, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TCLamnidis Nov 22, 2021

Uh oh!

jfy133 left a comment

Uh oh!

Uh oh!

jfy133 Nov 23, 2021

Uh oh!

jfy133 Nov 23, 2021

Uh oh!

jfy133 Nov 23, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	# picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT
	# picard -Xmx${task.memory.toGiga()}g AddOrReplaceReadGroups I=${libraryid}_seqtypemerged.bam O=${libraryid}_seqtypemerged_rg.bam RGID=1 RGLB="${libraryid}_seqtypemerged" RGPL=illumina RGPU=4410 RGSM="${libraryid}_seqtypemerged" VALIDATION_STRINGENCY=LENIENT

Conversation

TCLamnidis commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

github-actions Bot commented Nov 18, 2021

This PR is against the master branch ❌

Uh oh!

github-actions Bot commented Nov 18, 2021

Markdown linting is failing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TCLamnidis Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

jfy133 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jfy133 Nov 23, 2021

Choose a reason for hiding this comment

Uh oh!

jfy133 Nov 23, 2021

Choose a reason for hiding this comment

Uh oh!

jfy133 Nov 23, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

TCLamnidis commented Nov 18, 2021 •

edited

Loading

This PR is against the `master` branch ❌