scott-griffiths / bitstring

A Python module to help you manage your bits
https://bitstring.readthedocs.io/en/stable/index.html
MIT License
401 stars 67 forks source link

4.2.0: BitArray.join() no longer ignores None types #323

Closed Algafix closed 2 months ago

Algafix commented 3 months ago

Hello!

Thanks for the amazing library and for the continuous improvements, it is really helpful!

I've noticed that, since the 4.2.0 update, there is a behavioral change that breaks backward compatibility and is not reported in the version notes.

Previous to 4.2.0, when joining a list of BitArrays the None items were ignored, but now it tries to initialise them and raises an error.

l = [BitArray('0x2'), None, BitArray('0x3')]
BitArray().join(l)

bitstring == 4.1.4

BitArray('0x23')

bitstring == 4.2.1

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/lib/python3.8/site-packages/bitstring/bits.py", line 1479, in join
    s._addright(Bits._create_from_bitstype(item))
  File "/lib/python3.8/site-packages/bitstring/bits.py", line 132, in _create_from_bitstype
    b._setauto_no_length_or_offset(auto)
  File "/lib/python3.8/site-packages/bitstring/bits.py", line 518, in _setauto_no_length_or_offset
    raise TypeError(f"Cannot initialise bitstring from type '{type(s)}'.")
TypeError: Cannot initialise bitstring from type '<class 'NoneType'>'.

Thank you a lot!

scott-griffiths commented 3 months ago

Hi, thanks for the bug report.

I'll take a look at what's caused the change to see how intentional this change was. It's possible it's a side-effect of a good change that wasn't noticed or documented. If so I'd be tempted to call it an undocumented bug fix.

I'm not sure, but the TypeError trying to initialise a bitstring from None does feel like the right behaviour. If you have a good use case for needed to initialise from None then I'd be interested to see it.

Either way it's an undocumented change, so thanks for letting me know. I'll either fix it or document it as intentional in the next point release.

Algafix commented 3 months ago

For me, the current behavior makes more sense :)

Thanks!