nucleic / enaml

Declarative User Interfaces for Python
http://enaml.readthedocs.io/en/latest/
Other
1.53k stars 130 forks source link

Looper cannot refresh items when using a generator as the iterable #414

Closed frmdstryr closed 4 years ago

frmdstryr commented 4 years ago

This took me several hours to debug in a large application...

If the Looper is given a generator as it's iterable, eg, reversed or enumerate and the items are refreshed multiple times without the iterable actually being updated (and thus reset) the iterable reaches the end and removes all items added to the looper!

This reproduces it:

from enaml.core.api import Looper, Conditional
from enaml.widgets.api import Window, Container, Label, ObjectCombo

enamldef Main(Window): window:

    attr my_items = range(10)
    attr flag = 'a'
    attr show_labels: bool << flag == 'b' or flag == 'c'

    Container:
        ObjectCombo:
            items = ['a', 'b', 'c']
            selected := window.flag

        Conditional:
            condition << show_labels
            Label:
                text = 'Flag is B or C'

            # Nested loop on different attrs forces re-eval twice
            Conditional:
                condition << flag == 'c'
                Label:
                    text = 'Flag is C'
                Looper:
                    iterable << reversed(my_items)
                    Label:
                        text = str(loop_item)

It works fine if the combo is changed from a to b then c (it shows all labels from 0 to 9) but if you go straight from a to c the Looper removes all the items because the multiple levels of conditionals cause the looper to be refreshed multiple times without resetting the iterable back to the start.

I think to fix this iterable needs to be re-read from the expression engine every time it's refreshed.

MatthieuDartiailh commented 4 years ago

This is quite tricky since even if we were re-reading each time you could still get bitten by doing attr my_items = reversed(range(10)). One thing we could do is test that the iterable is safe to iter multiple times using the following:

it1 = iter(self.iterable)
it2 = iter(self.iterable)
if next(it1) != next(it2):
    raise ValueError("Iterable in Looper needs to be safe to iterate multiple times"

Testing this with range(10) works but fails with reversed(range(10))

Actually a better option would be to test that we get an iterable that is not an iterator using collections.abc.Iterator, that would be cleaner. Care to do a PR for that ?