Skip to content

Read evidence tweaks#72

Merged
timodonnell merged 2 commits intomasterfrom
read_evidence_tweaks2
Apr 29, 2015
Merged

Read evidence tweaks#72
timodonnell merged 2 commits intomasterfrom
read_evidence_tweaks2

Conversation

@timodonnell
Copy link
Copy Markdown
Contributor

  • fix bug that caused mishandling of soft clipped reads
  • add performance test script
  • some better assertions
  • other minor fixes

Due to the issue pysam-developers/pysam#106 these changes require a snapshot pysam until a release is made. Once pysam makes a new release I'll update the dependency to use that.

Review on Reviewable

 * fix bug that caused mishandling of soft clipped reads
 * add performance test script
 * some better assertions
 * other minor fixes
@timodonnell
Copy link
Copy Markdown
Contributor Author

Tests successful on both python 2.7 and 3.4 for this.

Comment thread test/read_evidence_performance.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.

I don't know if this is an insignificant style difference, but I usually define argument parsers at the global scope. Also, why do you need to get sys.argv[1:], wouldn't just parse_args() work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can put the parser at global scope.

By doing sys.argv[1:] myself I have a function that I could theoretically call from other code, like tests, without assuming anything about the global argv

timodonnell added a commit that referenced this pull request Apr 29, 2015
@timodonnell timodonnell merged commit 63059d8 into master Apr 29, 2015
@timodonnell timodonnell deleted the read_evidence_tweaks2 branch April 29, 2015 02:30
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.

2 participants