Error with FlowItems that have non-zero stretch or ortho_stretch

e-calder commented 3 years ago

Hi,

I've encountered the following error when using FlowItems with a stretch or ortho_stretch greater than 1.

The stack trace is below:

[2021-07-26 13:21:31.187] [ ERROR  ] '<' not supported between instances of 'QFlowWidgetItem' and 'QFlowWidgetItem'
C:\Users\[....]\site-packages\enaml\qt\q_flow_layout.py, line 891
  File "C:\Users\[....]\site-packages\enaml\qt\q_flow_layout.py", line 891, in setGeometry
    self._doLayout(self.contentsRect())
  File "C:\Users\[....]\site-packages\enaml\qt\q_flow_layout.py", line 955, in _doLayout
    res = self._doHorizontalLayout(rect, test)
  File "C:\Users\[....]\site-packages\enaml\qt\q_flow_layout.py", line 1027, in _doHorizontalLayout
    row.layout(x, curr_y)
  File "C:\Users\[....]\lib\site-packages\enaml\qt\q_flow_layout.py", line 360, in layout
    diffs.sort()

<class 'TypeError'>
'<' not supported between instances of 'QFlowWidgetItem' and 'QFlowWidgetItem'

Its a bit more complicated in my application but a simple repro is:

from enaml.widgets.api import Window, Container, FlowItem, FlowArea
from enaml.core.api import Include

enamldef SomeFlowItem(FlowItem):
    stretch = 1
    ortho_stretch = 1

enamldef Main(Window):
    Container:
        FlowArea:
            Include:
               objects << [SomeFlowItem() for _ in range(4)]

Now I haven't sent a PR as I don't know if this fix is the right approach but replacing:

diffs.sort()

at lines 360 and 576 of q_flow_layout.py with:

diffs.sort(key=lambda i: i[0])

seems to sort it, for my case at least.

I'd appreciate your input on this one please.

Thanks in advance,

Emy

MatthieuDartiailh commented 3 years ago

It is interesting we never hit that one before ! I believe this code relies on the ability that existed in Python 2 to order any object. However since it simply used pointer value for thing without a comparison operator, ignoring the item in diff seems perfectly reasonable to me. Could you make a PR and possibly add a test for this ?

@sccolbert do you agree with my diagnostic ?

sccolbert commented 3 years ago

@MatthieuDartiailh Your assessment is correct. The fix proposed in #448 looks good to me.

e-calder commented 3 years ago

Thanks both

MatthieuDartiailh commented 3 years ago

You are welcome @e-calder . Thanks for the PR

nucleic / enaml

Error with FlowItems that have non-zero stretch or ortho_stretch #447