-
Notifications
You must be signed in to change notification settings - Fork 84
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
Run containers part 2 #66
base: main
Are you sure you want to change the base?
Conversation
Awesome! I don't have much time to help atm but will review and comment where I can. :) |
src/bitmap/store.rs
Outdated
.map(|iv| iv.start..iv.end) | ||
.flatten() | ||
.collect(), | ||
intervals.iter().flat_map(|iv| iv.start..=iv.end).collect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh. Good catch.
c016371
to
4ae8986
Compare
11d5cda
to
cb69d80
Compare
d8e4923
to
183c1bb
Compare
85d5176
to
9744f12
Compare
I'm going to start picking this back up @Kerollmops. I noticed the |
Thanks a lot @josephglanville, indeed it seems like I didn't take enough time to make this method work. I hope you fix it, and wait for your PR, thank you 😄 |
@josephglanville I was taking a look through history in #12
I haven't made an opportunity to look through the code for this PR yet. Is that what were doing? My concern is if we deviate we wont be able to use the same "frozen" format for lazy containers |
when use insert_range in a empty RoaringBitmap , maybe can use RunContainer ? |
@saik0 yeah that is a valid concern and not something I had thought of at the time but think it makes sense as to make it easier to support mmaped bitmaps without needing separate code for them. I don't think the code is too hard to modify to use start + len instead just have to adjust when searching for intervals etc. If you want to work on this go ahead, my progress has been slow because I am (as usual) occupied with other things sadly. |
I'm unclear what's needed to finish this, is there anything I can do to help? |
I started working on the run containers implementation, my starting point is #56 by @josephglanville.
I fixed some bugs but I'm sure I introduced new ones with the operations on runs, I need to add a lot of new tests for runs.