ilanschnell / bitarray

efficient arrays of booleans for Python
Other
716 stars 98 forks source link

bitarray / frozenbitarray not registered as abc.MutableSequence / abc.Sequence #196

Closed scott-griffiths closed 1 year ago

scott-griffiths commented 1 year ago

They seem to have all of the required methods and mixin methods - just missing a registration line. Tested with Python 3.11 and bitarray 2.7.3.

>>> import bitarray
>>> bitarray.__version__
'2.7.3'
>>> from collections import abc
>>> a = bitarray.bitarray('0010')
>>> isinstance(a, abc.Sequence)
False
>>> isinstance(a, abc.MutableSequence)
False

Perhaps this is a regression as it was mentioned as fixed in Issue #49.

This isn't a problem for me but it was unexpected and thought you might like to know. It's easy enough to work around:

>>> abc.MutableSequence.register(bitarray.bitarray)
<class 'bitarray.bitarray'>
>>> isinstance(a, abc.MutableSequence)
True

Cheers.

ilanschnell commented 1 year ago

Thank you for bringing this issue to my attention. I guess the changes I did in the 1.5 release didn't fix #49, as you mentioned. I noticed that running

abc.MutableSequence.register(bitarray.bitarray)

will also make frozenbitarray an instance of MutableSequence (as it is a subclass bit bitarray). So I'm not sure how to fix the issue. That is, both bitarray and frozenbitarray are registered as Sequence, but only bitarray registered as MutableSequence. Do you have an idea?

scott-griffiths commented 1 year ago

Ah good point. The inheritance of bitarray and frozenbitarray is the wrong way round (so a frozenbitarray is an instance of bitarray but not vice versa). I can understand why it was done that way as the frozen class came much later, but I think it means that you'd have to hack it a bit...

I think it can be done by using a metaclass for frozenbitarray and overriding __instancecheck__ in that metaclass. I just gave it a try and it didn't do what I expected, but metaclasses confuse me.

I guess the simple half-solution is just to register as Sequence.

ilanschnell commented 1 year ago

Thanks for the quick response. Yes, if bitarray would be a subclass of frozenbitarray, things would work without tweaks. isinstance(a, abc.Hashable) works out of the box, but that doesn't turn frozenbitarray object automatically into Sequence instances (when bitarray is registered as MutableSequence). PEP 3119 gives an example on how ABC Metaclasses are used: https://peps.python.org/pep-3119/ I'll look more into it when I have time. Thanks again the reporting the issue, and the pointers.

ilanschnell commented 1 year ago

I've decided to register bitarray with abc.MutableSequence. This way, bitarray is an instance of abc.MutableSequence, abc.Sequence and not abc.Hashable. So for bitarray everything is fine, which I think is more important than frozenbitarray. I consider the fact that frozenbitarray is also an instance of abc.MutableSequence a known bug (but this was bug before as well). I've tried to fix it with __isinstancecheck__, but it didn't work, maybe related to https://bugs.python.org/issue45791

ilanschnell commented 1 year ago

Closing issue as fix is now available in 2.7.4 release. Note that as bitarray is registered with abc.MutableSequence, frozenbitarray (as it inherits from bitarray) is also registered as a MutableSequence (which seems tricky to fix). This is considered a known bug.