Skip to content
This repository has been archived by the owner on Jan 27, 2020. It is now read-only.

Remapping #717

Merged
merged 17 commits into from
Feb 20, 2019
Merged

Remapping #717

merged 17 commits into from
Feb 20, 2019

Conversation

maxulysse
Copy link
Member

@maxulysse maxulysse commented Jan 17, 2019

  • Add new remapping step (possibility to remap BAMs)
  • fastqFiles renamed to inputFiles
  • MapReads for remapping will now convert BAM to FASTQ and feed it to BWA on the fly

PR checklist

  • PR is made against dev branch
  • This comment contains a description of changes (with reason)
  • If you've fixed a bug or added code that should be tested, add tests!
  • Ensure the test suite passes (./scripts/test.sh -p docker -t ALL).
  • Documentation in docs is updated
  • CHANGELOG.md is updated

@maxulysse
Copy link
Member Author

I think we could improve that by checking if input is bam or fastq, and then do the correct thing

main.nf Outdated Show resolved Hide resolved
@maxulysse
Copy link
Member Author

I'm now happy with my latest changes.
@alneberg @szilvajuhos if you want to comment, approve or not

@maxulysse
Copy link
Member Author

The only thing that is really missing is tests for this new feature...
But as this is an edge case, I'm wondering if we should test it or not.
What do you think?


output:
file "*_fastqc.{zip,html}" into fastQCreport

when: step == 'mapping' && !params.noReports

script:
inputFiles = SarekUtils.hasExtension(inputFile1,"fastq.gz") ? "${inputFile1} ${inputFile2}" : "${inputFile1}"
Copy link
Member

Choose a reason for hiding this comment

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

We should probably allow fq.gz as well right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can think of it in the extractSample() function, but currently I can't think of anyone asking me about this particular .fq.gz extension.

Copy link
Collaborator

@szilvajuhos szilvajuhos left a comment

Choose a reason for hiding this comment

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

What about the BAM index files in the tsv file example?

@maxulysse
Copy link
Member Author

The index is not in the TSV when mapping BAMs.
I figured that if you want to (re)map BAMs, there's little use of the indexes.

Copy link
Collaborator

@szilvajuhos szilvajuhos left a comment

Choose a reason for hiding this comment

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

Looks OK

@szilvajuhos szilvajuhos merged commit e331795 into SciLifeLab:dev Feb 20, 2019
@maxulysse maxulysse deleted the remapping branch February 20, 2019 09:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants