Fix has_overlap/get_overlap for flipped (reverse-strand) coordinates (2.0.6) - #42
Merged
Conversation
Reverse-strand features sometimes pass genomic coordinates as (end, start) rather than (start, end) -- has_overlap silently returned False for partial overlaps in that case, causing add_domains_to_table to miss overlapping domains on the reverse strand with no warning. Normalize via explicit min/max on just the first two elements (not the whole object, since callers pass intervaltree.Interval objects with extra data too). Closes #23.
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #23:
add_domains_to_tablesilently missed overlapping domains on reverse-strand genes with no warning.Root cause (as diagnosed in detail by the reporter):
has_overlap/get_overlapassumedstart <= end, butgenomic_positionproduces(end, start)for reverse-strand features, since transcript-relative positions map in the opposite direction to genomic coordinates on the minus strand.has_overlap((118, 90), (100, 200))wrongly returnedFalsefor a real partial overlap.Fixed at the primitive level (
has_overlap/get_overlapin_utils.py) rather than atgenomic_position's output, since these two functions are called from ~15 places across the codebase and normalizing there is the safest, most general fix -- it's a no-op for already-correctly-ordered pairs, and defensively hardens every other caller against the same class of bug too.Found and fixed a real regression in my own first attempt: some callers (e.g.
IntervalArray.overlapin_transcriptome_io.py) passintervaltree.Intervalobjects, which behave like 3-element structures (begin, end, data_dict). The original code only ever indexed[0]/[1]; my first fix calledmin()/max()over the whole object, which crashed on the data dict. Caught by running the full test suite (not just the new test) before pushing -- fixed by indexing explicitly instead.Added
tests/utils_test.py(new file, following the file-per-feature-area convention documented intests/README.md) with the reporter's exact reproducer cases. Verified it reproduces the original wrong result against the pre-fix code.Bumps version to 2.0.6.
Test plan
pytestsuite passes (18 passed), including the new regression testflake8/blackcleanpython -m build+twine check dist/*pass at 2.0.6