-
Notifications
You must be signed in to change notification settings - Fork 9
#21 added merge_overlaps_with method #23
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
base: master
Are you sure you want to change the base?
#21 added merge_overlaps_with method #23
Conversation
Hi @sstadick - I was looking for this feature for my use case and found myself here, happy to help push this across the finish line, but am curious what you'd like to see here to merge this. |
Hi @cademirch! I'll go back and take a look at what's here this weekend, I don't remember what held this back the first time around. Do you just need the merge_with? Or some of the other stuff that was added as well? |
@sstadick No rush, thanks! I just need merge_with. For context, I am wanting to collect all of the overlapping intervals' values in the merged interval (unless this is possible already?). |
@cademirch @bennobuilder - finally read through all this, and it's great! I'm really sorry I missed this when it originally went up. I'm down to get it merged, it just needs the rebase conflicts merged. I do have one question - for the merge_with I would kind of lean toward passing in the full |
@sstadick great! I'm heading on vacation for a few days but can help with this when I'm back. As for passing the full interval vs value, I'm not sure how much it matters to my use case, but can look more when I'm back. |
Closes #21