adafruit / Adafruit_CircuitPython_HT16K33

Adafruit CircuitPython driver for the HT16K33, a LED matrix driver IC.
MIT License
41 stars 29 forks source link

colon @property on Seg7x4 hides multiple colon access on BigSeg7x4 #98

Closed angerer closed 2 years ago

angerer commented 2 years ago

The @property that was added to the Seg7x4 class to mediate access to the self.colon = Colon() class member is interfering with using the member directly on the BigSeg7x4 subclass. The @property methods are being inherited from the superclass and only the self.colon[0] item can be accessed.

I'm happy to put together a pull request, but wasn't sure if you'd want to remove the @property methods from the superclass or override them in the subclass. If the property methods are going to be left, I would expect the self.colon member of the class to be renamed to self._colon to help clear up confusion about accessing it directly. (Though, I come from a Java background and I'm still learning the Python idoms and that might be incorrect.)

angerer commented 2 years ago

I did some playing around trying to recreate this outside of my main project where I encountered the error. So far, I have learned:

import board
from adafruit_ht16k33.segments import BigSeg7x4

i2c = board.I2C()
bigdisplay = BigSeg7x4(i2c)
bigdisplay.colon[1] = True

results in:

Traceback (most recent call last):
  File "./7seg_property_test.py", line 10, in <module>
    bigdisplay.colon[1] = True
TypeError: 'bool' object does not support item assignment
dhalbert commented 2 years ago

@makermelissa This is related to your #56.

The subclassing could be redone so that class BigSeg7x4(Seg7x4) is no longer the relationship, but instead there is an AbstractSeg7x4 that contains the majority of the implementation, and then colon is specialized.

I am also thinking maybe it should be Colons and .colons for the general case, but that is an incompatible change. However, the Seg7x4.colon helper would still work, so that might be fine.