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

Fix error that occurs when querying intervals for missing chr #247

Merged
merged 1 commit into from
Sep 29, 2021

Conversation

athos
Copy link
Member

@athos athos commented Sep 29, 2021

When issuing an interval query (using c.u.i/find-overlap-intervals) for a missing chr in the index, an error will be thrown:

(require '[cljam.util.intervals :as intervals])

(def index
  (intervals/index-intervals
   [{:chr "chr1", :start 10000, :end 15000}
    {:chr "chr1", :start 20000, :end 25000}]))

(intervals/find-overlap-intervals index "chr1" 13000 17000)
;=> ({:chr "chr1", :start 10000, :end 15000})
(intervals/find-overlap-intervals index "chr1" 17000 19000)
;=> ()

(intervals/find-overlap-intervals index "chr2" 13000 17000)
;; Execution error (IllegalArgumentException) at cljam.util.intervals/eval5662$fn$G (intervals.clj:4).
;; No implementation of method: :find-overlap-intervals* of protocol: #'cljam.util.intervals/IIntervals found for class: nil

This PR fixes the error by guarding against nil with some->.

@athos athos requested review from alumi and niyarin September 29, 2021 09:21
@athos athos requested a review from a team as a code owner September 29, 2021 09:21
@athos athos removed the request for review from a team September 29, 2021 09:24
Copy link
Member

@alumi alumi left a comment

Choose a reason for hiding this comment

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

Wow, thank you for the fix! 👍

Copy link
Contributor

@niyarin niyarin left a comment

Choose a reason for hiding this comment

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

Thank you for fixing. LGTM 👍
(I wrote the original code , but I overlooked the case with no corresponding chr.)

@niyarin niyarin merged commit 582407c into master Sep 29, 2021
@niyarin niyarin deleted the fix/query-for-missing-chr branch September 29, 2021 22:04
@athos
Copy link
Member Author

athos commented Sep 30, 2021

@alumi @niyarin Thanks for the quick review!

@alumi Do you think it's worth cutting another release soon?

@alumi
Copy link
Member

alumi commented Sep 30, 2021

Sure! I'll release 0.8.2 in an hour 👍

@athos
Copy link
Member Author

athos commented Sep 30, 2021

Thanks a lot 🙏 I'm looking forward to it!

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.

None yet

3 participants