PyRoaringBitMap icon indicating copy to clipboard operation
PyRoaringBitMap copied to clipboard

Implement "in place" binary operators on `FrozenBitMap` (to match `frozenset`)

Open PeterJCLaw opened this issue 3 years ago • 3 comments

For example:

In [1]: a = ref = frozenset(range(3))

In [2]: a
Out[2]: frozenset({0, 1, 2})

In [3]: a &= frozenset([3, 1, 4])

In [4]: a
Out[4]: frozenset({1})

In [5]: ref
Out[5]: frozenset({0, 1, 2})

(Also for |=, ^= and possibly others)

It might be good if FrozenBitMap supported the same API.

PeterJCLaw avatar Oct 07 '22 09:10 PeterJCLaw

I think my initial thought was to explicitly forbid these operations (see frozen_bitmap.pxi) to avoid any confusion. But you are right that it might be better to respect the "standard" API.

Would you like to do a pull request?

Ezibenroc avatar Oct 12 '22 07:10 Ezibenroc

Ah, I'd not spotted that this was intentional. I agree the "standard" API has the potential for confusion. I do suspect it may be the lesser of two evils to follow it though (I don't feel strongly).

I'd definitely be up for this, though may not have time for a couple of weeks.

PeterJCLaw avatar Oct 12 '22 08:10 PeterJCLaw

Sure, whenever you want!

I did not test, but it might be enough to simply remove the methods from the FrozenBitMap. I think that if the in-place methods (like __iand__) are not implemented, Python automatically calls the not in-place version (like __and__). Modifying the tests will take a bit longer, but this should be straightforward (e.g. something like the piece of code you wrote).

Ezibenroc avatar Oct 12 '22 09:10 Ezibenroc