Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- Make Unique intervals keep both interval names even when intervals … #1265

Merged
merged 1 commit into from
Jan 24, 2019

Conversation

yfarjoun
Copy link
Contributor

…are the same (assuming names are different)

  • added a test

Description

Please explain the changes you made here.
Explain the motivation for making this change. What existing problem does the pull request solve?

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)

@codecov-io
Copy link

codecov-io commented Jan 23, 2019

Codecov Report

Merging #1265 into master will increase coverage by 0.004%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #1265       +/-   ##
===============================================
+ Coverage     69.353%   69.357%   +0.004%     
- Complexity      8301      8302        +1     
===============================================
  Files            555       555               
  Lines          33116     33117        +1     
  Branches        5572      5572               
===============================================
+ Hits           22967     22969        +2     
+ Misses          7890      7886        -4     
- Partials        2259      2262        +3
Impacted Files Coverage Δ Complexity Δ
...c/main/java/htsjdk/samtools/util/IntervalList.java 68.437% <100%> (+0.981%) 71 <5> (ø) ⬇️
...samtools/util/AsyncBlockCompressedInputStream.java 72% <0%> (-4%) 12% <0%> (-1%)
src/main/java/htsjdk/samtools/util/Interval.java 61.818% <0%> (ø) 25% <0%> (+1%) ⬆️
...va/htsjdk/samtools/sra/SRAIndexedSequenceFile.java 64.865% <0%> (+2.703%) 7% <0%> (+1%) ⬆️

…are the same (assuming names are different)

- added a test
@yfarjoun yfarjoun force-pushed the yf_keep_both_interval_names_on_merge branch from 19e57c6 to e38e4bc Compare January 24, 2019 17:17
Copy link
Member

@lbergelson lbergelson left a comment

Choose a reason for hiding this comment

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

Looks good to me.

int start = intervals.first().getStart();
int end = intervals.last().getEnd();
final boolean neg = intervals.first().isNegativeStrand();
static Interval merge(final Iterable<Interval> intervals, final boolean concatenateNames) {
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 consider lifting it into Locatable as something like getSpanningLocatable. We'd have to decide on a concrete representation to instantiate though, so maybe that should wait until a different pr.

@lbergelson lbergelson merged commit 3c48018 into master Jan 24, 2019
@lbergelson lbergelson deleted the yf_keep_both_interval_names_on_merge branch January 24, 2019 20:25
lbergelson added a commit that referenced this pull request Mar 6, 2019
* Fixing a bug in IntervalList.merge() that caused it to break when merging 0 length intervals
* Improving tests for merge
* The bug was introduced in #1265
lbergelson added a commit that referenced this pull request Mar 6, 2019
* Fixing a bug in IntervalList.merge() that caused it to break when merging 0 length intervals
* Improving tests for merge
* The bug was introduced in #1265
lbergelson added a commit that referenced this pull request Mar 7, 2019
* Fixing a bug in IntervalList.merge() that caused it to break when merging 0 length intervals
* Improving tests for merge
* The bug was introduced in #1265
lbergelson added a commit that referenced this pull request Mar 8, 2019
* Fixing a bug in IntervalList.merge() that caused it to break when merging 0 length intervals
* Improving tests for merge
* The bug was introduced in #1265
lbergelson added a commit that referenced this pull request Mar 11, 2019
* Fixing zero-length interval bug in IntervalList.merge

* Fixing a bug in IntervalList.merge() that caused it to break when merging 0 length intervals
* Improving tests for merge
* The bug was introduced in #1265

* improving comment in IntervalList
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