Skip to content

Adding tests revealing 0-length interval nonsense - #1320

Draft
lbergelson wants to merge 1 commit into
masterfrom
lb_zero_length_interval_tests
Draft

Adding tests revealing 0-length interval nonsense#1320
lbergelson wants to merge 1 commit into
masterfrom
lb_zero_length_interval_tests

Conversation

@lbergelson

Copy link
Copy Markdown
Member

This PR adds failing tests that test intersection/abutment using 0-length intervals.

Many are failing...

I've added an ascii diagram in the test to show my belief of what the various intervals represent.

The equivalent methods in Interval and Locatable have slightly different failure modes.

We should decide what the right answer is, and then fix the various methods to implement those.

We should also check related methods like encloses and overlap in CoordMath.

I'm sick of this issue and want it to be resolved so we never have to think about it again.

Adding tests that test intersection/abutment using 0-length intervals.
Many are failing...
I've added an ascii diagram in the test to show my belief of what the various intervals represent.
Zero length intervals don't intersect themselves.  Should they?

The equivalent methods in Interval and Locatable have slightly different failure modes.
{chr1_22_30, zeroLength_chr1_22_21, true, false},
{chr1_23_30, zeroLength_chr1_22_21, false, true},
{chr1_24_30, zeroLength_chr1_22_21, false, false},
{zeroLength_chr1_22_21, zeroLength_chr1_22_21, true, false},

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.

hmmm. to answer his question we need a definition of "intersects" and "abuts"

Intersects: There exists at least one base in the reference that is present in both intervals.
Abuts: Two intervals that do not intersect (c.f.) , however, the union of bases they cover can be written as one interval.

According to this definition, two empty intervals in the same position do not intersect, but they do abut.

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.

we have both intersects and overlaps? huh?

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.

it's the same function, implemented twice!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We have more in other places I'm sure. Because of fear or weird details like this we never merged the implementations of a lot of these.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants