enthought / traitsui

TraitsUI: Traits-capable windowing framework
http://docs.enthought.com/traitsui
Other
297 stars 96 forks source link

Bug: Tree editor does not update in the UI #1978

Open wilsonnater opened 1 year ago

wilsonnater commented 1 year ago

I've attached a semi-minimal example, but a tree editor item does not update in the UI when data is added to the object if the tree editor has hide_root=True, the item is style='custom' and has a visible_when criteria, and the Group has scrollable=True.

I am not seeing any error message when running this code.

Semi-Minimal example ``` from traits.api import Instance, List, Str, Button, Bool, HasTraits from traitsui.api import ( Group, Item, ModelView, TreeEditor, TreeNode, View, UItem,) class Data_Group(HasTraits): name = Str class Holder(HasTraits): name = Str Data_Groups = List(Instance(Data_Group)) class SimpleExample(ModelView): model = Instance(Holder) # Adds another Data_Group button = Button('Add') def _button_fired(self): yeah = Data_Group(name='frank') self.model.Data_Groups.append(yeah) def default_traits_view(self): tree_editor = TreeEditor( nodes=[ TreeNode( node_for=[Holder], auto_open=True, children='Data_Groups', label='', ), TreeNode( node_for=[Data_Group], auto_open=False, label='name', ), ], # If hide_root is commented out(or set to false) it works hide_root=True, orientation='vertical', ) return View( Group( Item( name='model', editor=tree_editor, resizable=True, show_label=False, ), Item('button'), show_border=True, ), title='Measurement Groups', width=500, resizable=True, ) class SimpleView(ModelView): model = Instance(SimpleExample) show_measurement_groups_table = Bool() def default_traits_view(self): return View( Group( Item(name='show_measurement_groups_table', ), UItem( 'model', # If style is commented out it works style='custom', # If visible_when is commented out it works visible_when='show_measurement_groups_table', ), ), resizable=True, # If scrollable is commented out it works scrollable=True, ) if __name__ == '__main__': temp = Holder(name='test') collection = SimpleExample(model=temp) mcv = SimpleView(model=collection) mcv.configure_traits() ```
corranwebster commented 1 year ago

I can verify this behaviour on Python 3.8, PySide6, MacOS.

Edit: ~It looks like listeners are not being hooked up on the root node.~ Nope - that was a problem with my instrumentation of the code. Edit 2: I have run out of time to look at this today - will try to dig a bit deeper in the next few days.

corranwebster commented 1 year ago

This has some of the feel of https://github.com/enthought/traitsui/pull/1303 and related issues, but I thought that those had been resolved.

corranwebster commented 1 year ago

Digging in, when asking for the expanded children, etc. of the hidden node, we are getting a swallowed exception: Qt object <PySide6.QtWidgets.QTreeWidgetItem object at 0x121365080> for node nolonger exists.

Edit: so it looks like the QTreeWidgetItem in question is being created before the tree is shown, but being deleted at some point before things are updated. Whether or not this is an issue, the problem would seem to be in part that when there is a stale tree node we may not want to ignore it. The question is then whether the node is stale because the tree editor is being finalized (in which case it would be bad to try and re-create it), or because something else happened.

Relevant code is here: https://github.com/enthought/traitsui/blob/d944a86a33b31eebe3fa6a2b6828077c12bc8b09/traitsui/qt4/tree_editor.py#L645-L649

corranwebster commented 1 year ago

The above code is recent code, but without the try/except we would have a segfault instead of nothing happening, I suspect.

As best I can tell at the moment, this looks like it may be either a bug in Qt or a problem with the way that we use QTreeWidgetItem objects (ie. the expectations we have on their lifetime and structure). Either way, this feels like it will require a bigger change than we can fit into the next bugfix release (either to work-around or fix).

wilsonnater commented 1 year ago

If it will take a while it would probably be better to focus on the next release as that will used in metrology. This problem can be worked around with minimal changes to the UI by setting hide_root=False