-
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
contains_range / range_cardinality functions #237
Conversation
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.
Hey, thank you very much for that, it looks good to me. But would it be possible to add a quickcheck test that:
- One that creates a RoaringBitmap
/Treemapwith a random range, adds more numbers into it (in the front and the back of that range) and check that the original range it contained in it. - One that does the same but we remove some random numbers from inside this range and check that the range is not contained.
Note that I just renamed the master branch to main, you should be careful. |
Good call, found a place where I had an off-by-one in the range_cardinality function. |
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.
Thank you very much for your work and those new tests.
bors merge
Build succeeded: |
237: contains_range / range_cardinality functions r=Kerollmops a=Dr-Emann Fixes RoaringBitmap#235 Co-authored-by: Zachary Dremann <[email protected]>
Fixes #235