Added a secondary constructor to IntervalList that takes a dict. #703

Merged
merged 1 commit into from Sep 13, 2016

Conversation

Projects
None yet
4 participants
Owner

tfenne commented Sep 12, 2016

Description

I find myself writing these three lines of code again and again and again to initialize an IntervalList since it requires a SAMFileHeader with a useful SAMSequenceDictionary and there's no way to create a header from a dictionary as an expression.

Checklist

  • Code compiles correctly
  • New tests covering changes and new functionality
  • All tests passing
  • Extended the README / documentation, if necessary
  • Is not backward compatible (breaks binary or source compatibility)

Coverage Status

Coverage decreased (-0.01%) to 68.908% when pulling aad6c66 on tf_interval_list_from_sd into c3d5a88 on master.

Contributor

magicDGS commented Sep 12, 2016

Why a private method in IntervalList and not a public static in IntervalUtil. It may be useful not only for construct an interval list...

Owner

tfenne commented Sep 12, 2016

@magicDGS I wanted the secondary constructor there. It's ~3 lines of simple code - I don't think it warrants extracting to a separate utility class.

Owner

tfenne commented Sep 12, 2016

Also, yay, SRA tests are failing.

droazen self-assigned this Sep 13, 2016

Contributor

droazen commented Sep 13, 2016 edited

I've found myself sometimes having to go from SAMSequenceDictionary -> SAMFileHeader as well in other contexts -- perhaps you could add a new constructor to SAMFileHeader that takes a SAMSequenceDictionary, and call into that constructor from your new IntervalList constructor?

Also, add a simple unit test so we have test coverage on the new constructor(s).

Contributor

droazen commented Sep 13, 2016

Review complete, back to @tfenne

@droazen droazen assigned tfenne and unassigned droazen Sep 13, 2016

Owner

tfenne commented Sep 13, 2016

Sure @droazen I can do that instead. I wasn't sure there were other uses for it. I'll swap that around then merge.

@tfenne tfenne Added a secondary constructor to IntervalList that takes a dict.
8fb8a7a

Coverage Status

Coverage decreased (-0.009%) to 68.984% when pulling 8fb8a7a on tf_interval_list_from_sd into 21d8865 on master.

@tfenne tfenne merged commit 7886768 into master Sep 13, 2016

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.009%) to 68.984%
Details

tfenne deleted the tf_interval_list_from_sd branch Sep 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment