Skip to content

Add --output-orphaned option to split-paired-reads.py#1164

Merged
mr-c merged 20 commits intomasterfrom
feature/split-option
Aug 5, 2015
Merged

Add --output-orphaned option to split-paired-reads.py#1164
mr-c merged 20 commits intomasterfrom
feature/split-option

Conversation

@camillescott
Copy link
Copy Markdown
Member

Currently, so far as I can tell, there's no way to both split paired reads into .1 and .2 files while also outputting orphans to their own file without doing messy pipe stuff. This adds that feature to split-paired-reads.py with an extra command line option.

@ctb
Copy link
Copy Markdown
Member

ctb commented Jul 10, 2015

This should fix #847, right?

@ctb
Copy link
Copy Markdown
Member

ctb commented Jul 19, 2015

@camillescott may I snarf this?

@ctb
Copy link
Copy Markdown
Member

ctb commented Jul 20, 2015

ping @camillescott

@ctb
Copy link
Copy Markdown
Member

ctb commented Jul 23, 2015

Taking it and making it my own.

@ctb ctb added this to the 2.0 milestone Jul 29, 2015
@mr-c
Copy link
Copy Markdown
Contributor

mr-c commented Jul 30, 2015

LGTM

@ctb
Copy link
Copy Markdown
Member

ctb commented Jul 31, 2015

…option

Conflicts:
	scripts/split-paired-reads.py
@ctb
Copy link
Copy Markdown
Member

ctb commented Aug 1, 2015

Update - removed filename option with -0, since

  split-paired-reads.py -0 <file>

was ambiguous.

  • test -0 with filename
  • test -p and evaluate error message
  • compression stuff (bzip2/gzip)

@ctb
Copy link
Copy Markdown
Member

ctb commented Aug 4, 2015

  • Is it mergeable?
  • Did it pass the tests?
  • If it introduces new functionality in scripts/ is it tested?
    Check for code coverage with make clean diff-cover
  • Is it well formatted? Look at make pep8, make diff_pylint_report,
    make cppcheck, and make doc output. Use make format and manual
    fixing as needed.
  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.
  • Is it documented in the ChangeLog?
    http://en.wikipedia.org/wiki/Changelog#Format
  • Was a spellchecker run on the source code and documentation after
    changes were made?
  • Is the Copyright year up to date?

Ready for review @mr-c @bocajnotnef @luizirber

@ctb
Copy link
Copy Markdown
Member

ctb commented Aug 4, 2015

Fixes #847.

Comment thread ChangeLog Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you indent this to match your name?

Comment thread scripts/split-paired-reads.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not use args.output_orphaned everywhere you check allow_orphans?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On Tue, Aug 04, 2015 at 09:52:19AM -0700, Michael R. Crusoe wrote:

@@ -128,47 +131,48 @@ def main():
# Use default filename created above
fp_out2 = get_file_writer(open(out2, 'wb'), args.gzip, args.bzip)

  • put orphaned reads here, if -0!

  • allow_orphans = False

Why not use args.output_orphaned everywhere you check allow_orphans?

It's partly a leftover of some now-removed logic that permitted -0 without
a filename, but what made me keep it in was this code:

require_paired=not allow_orphans

which I think would be uglier and less clear if it was
not args.output_orphaned. What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm okay with not args.output_orphaned

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On Tue, Aug 04, 2015 at 03:54:24PM -0700, Michael R. Crusoe wrote:

@@ -128,47 +131,48 @@ def main():
# Use default filename created above
fp_out2 = get_file_writer(open(out2, 'wb'), args.gzip, args.bzip)

  • put orphaned reads here, if -0!

  • allow_orphans = False

I'm okay with not args.output_orphaned

That doesn't address "here's my reasoning, what do you think" but I'll give
it a whirl ;)

@mr-c
Copy link
Copy Markdown
Contributor

mr-c commented Aug 4, 2015

Otherwise, this LGTM

@ctb
Copy link
Copy Markdown
Member

ctb commented Aug 4, 2015

@ctb
Copy link
Copy Markdown
Member

ctb commented Aug 5, 2015

Fixes #1188.

Ready for review @mr-c.

mr-c added a commit that referenced this pull request Aug 5, 2015
Add --output-orphaned option to split-paired-reads.py
@mr-c mr-c merged commit e35c750 into master Aug 5, 2015
@mr-c mr-c deleted the feature/split-option branch August 5, 2015 16:55
@mr-c
Copy link
Copy Markdown
Contributor

mr-c commented Aug 5, 2015

Great, thanks!

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.

3 participants