Add option to specify UMI separator in extract 2

CGATOxford / UMI-tools

Tools for handling Unique Molecular Identifiers in NGS data sets

MIT License

481 stars 190 forks source link

Add option to specify UMI separator in extract 2 #549

Closed janoppelt closed 2 years ago

janoppelt commented 2 years ago

Add option to specify UMI separator in umitools extract. The default stays as it is now (""). Allows users to change it to other strings. 2

IanSudbery commented 2 years ago

Hi @TomSmithCGAT can we set up the CI to run on PRs as well as commits?

@opplatek This looks good, but I'll need to run our tests.

TomSmithCGAT commented 2 years ago

I just need to finish off #546 (add py 3.5) and then merge and it should run on all pushes

https://github.com/CGATOxford/UMI-tools/blob/cd6cfa72adabfe8a8e8b1ea97bd4653ffa9cfec0/.github/workflows/nosetests.yml#L3

I've blocked out tomorrow for just UMI-tools so I'll do this first thing and then try and resolve the deterministic issues

TomSmithCGAT commented 2 years ago

Merged #546 and set to run CI on PRs and pushes. Need to work out how to get it to run on an existing PRs now though... If all else fails, I guess it's a re-issue PR or add new commit with e.g empty line

TomSmithCGAT commented 2 years ago

💡 Just need to close and re-open and github treats it as a new PR 😁

TomSmithCGAT commented 2 years ago

@opplatek - Am I right in understanding that the two PRs from your patch-1 and patch-2 branches both need to be merged? I don't think the testing is appropriate for the separate PRs:

patch-1 fails the tests because of an issue resolved in patch-2
patch-2 passes the tests because it doesn't have any effect without the changes in patch-1

Could you please merge the two patches and issue a new PR 🙏 Thank you!

janoppelt commented 2 years ago

Hi @TomSmithCGAT . Sorry for the confusion. The new push of the changes is at #551.