displayio: Empty Group does not allow insert

[kmatch98]

In debugging an issue with Label, I uncovered an unexpected error when using insert with an empty Group. Normal lists allow insert to be used on an empty list and the function is effectively equivalent to append. However when trying to insert with an empty displayio.Group, an unexpected error IndexError: Group index out of range is triggered. Due to this unexpected behavior a kludge to is required overcome this by first checking the size of the Group and then use append if the length is zero.

Here is a command line experiment that shows the bug where insert doesn't work with an empty Group:

import displayio
myGroup=displayio.Group()
myGroup2=displayio.Group()
myGroup.insert(0, myGroup2)

Here is the error that I received:

>>> myGroup.insert(0, myGroup2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: Group index out of range
>>> 

[tannewt]

Want to try fixing it? I think you'll need to add an exception here: https://github.com/adafruit/circuitpython/blob/main/shared-bindings/displayio/Group.c#L210 or maybe the mp_get_index call can be modified. I think it's throwing the error.

[kmatch98]

I took a look at this function and also in obj.c for the definition of mp_get_index and I think you are right that the error is caused here because theindex which is Zero is >= the length of the empty list len (See below code snippet from obj.c line 423).

As I’m totally unfamiliar with the usage reasons for mp_get_index (It looks like it is doing some kind of type conversion), I would suggest updating the insert function to only call mp_get_index if the list length is > 0.

I’ve never built CircuitPython before so I’ll be on a steep part of the learning curve. Let me look at the tutorial on building CP and see if I’m willing to take a crack at it.

See line 423 in obj.c:

    } else {
        if (i < 0 || (mp_uint_t)i >= len) {
            if (MICROPY_ERROR_REPORTING == MICROPY_ERROR_REPORTING_TERSE) {
                mp_raise_IndexError(translate("index out of range"));
            } else {
                mp_raise_msg_varg(&mp_type_IndexError,
                    translate("%q index out of range"), type->name);
            }
        }

[tannewt]

mp_get_index is used to convert indices like -1 to the actual index. I think the exception needs to be if insert index is equal to the length of the group.

Ping me on Discord if you have any questions about building CircuitPython.

[kmatch98]

Closed with merge above.