Skip to content

Add Read Groups to bowtie2 mapping#656

Closed
IdoBar wants to merge 6 commits intonf-core:devfrom
IdoBar:patch-1
Closed

Add Read Groups to bowtie2 mapping#656
IdoBar wants to merge 6 commits intonf-core:devfrom
IdoBar:patch-1

Conversation

@IdoBar
Copy link
Copy Markdown
Contributor

@IdoBar IdoBar commented Jan 10, 2021

Add Read Group information to bowtie2 mapping to fix issue [#655 ]

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - add to the software_versions process and a regex to scrape_software_versions.py
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/eager branch on the nf-core/test-datasets repository.
  • 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).

jfy133 and others added 2 commits December 9, 2020 08:48
Add Read Group information to `bowtie2` mapping to fix issue [nf-core#655 ]
@github-actions
Copy link
Copy Markdown

Hi @IdoBar,

It looks like this pull-request is has been made against the IdoBar/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 IdoBar/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.

Thanks again for your contribution!

@IdoBar IdoBar changed the base branch from master to dev January 10, 2021 23:35
@jfy133
Copy link
Copy Markdown
Member

jfy133 commented Jan 11, 2021

Hi @IdoBar

Thanks very much for the PR! This looks great (and you passed all the CI tests 💪 ), and thanks for catching the issue.

Your change looks good - thanks for catching that. I made tiny tweak for consistency with bwa, and added a CI test for this particular case. I made a PR into the patch on your fork for this.

If you could merge that into your fork's branch - this should trigger the auto tests again here. Once they pass, we can update the changelog and merge.

Only thing is we are in the middle of a new release (probably today), so this fix will need to be merged in the first patch release after this. So I'll let you know once the release is done and you can update the changelog and I can merge for you.

jfy133 and others added 2 commits January 11, 2021 06:28
Add CI test for bowtie2 readgroup issue
@IdoBar
Copy link
Copy Markdown
Contributor Author

IdoBar commented Jan 11, 2021

Hi @jfy133,
It took me a while (I'm still trying to get my head around these GH workflows...), but I think that I managed to merge your PR.
Happy to contribute and I'm looking forward to see it implemented.
Thanks, Ido

@jfy133
Copy link
Copy Markdown
Member

jfy133 commented Jan 11, 2021

PR merge looks good 👍 . I will hopefully release 2.3 today and then we can start preparing the patch with this fix today!

Comment thread main.nf Outdated
Comment thread main.nf Outdated
IdoBar added a commit to IdoBar/eager that referenced this pull request Jan 11, 2021
@jfy133
Copy link
Copy Markdown
Member

jfy133 commented Jan 12, 2021

Closing for #658

@jfy133 jfy133 closed this Jan 12, 2021
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.

Read Group not added to bam files generated by bowtie2, which causes GATK genotyping to fail

2 participants