Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TreeView/TableView/ListView in conjuction with psygnal EventedDataclass/EventedModel #255

Open
ndxmrb opened this issue Aug 16, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@ndxmrb
Copy link

ndxmrb commented Aug 16, 2024

Hi all,
I wasn't sure where to place this question (which might lead to a feature request?), but because of #77 I now decided to go ahead here:

If I had a psygnal EventedContainer holding EventedModel instances, I'd like those to be shown in a QTable[...] or even a QTree[...].
Two approaches are feasible:

  1. Create a ModelMapper that maps the EventedContainer to a QAbstractItemModel. This could utilize the .internalPointer() function to return data directly from the EventedModel. Then you could use a QTreeView or QTableView to show the data from the Qt model.
  2. Create a QTreeWidget or QTableWidget, in which the respective Items are connected to the EventedModel signals.

I see the following issues with these approaches:

  1. Maybe you could move the entire EventedModel to a separate thread, such that data operations always run there and never in the GUI thread. Maybe the Qt model approach prohibits that because of the pointer to the data? It's stated multiple times in Qt forums that you should never ever touch model data from outside the GUI thread.
  2. Probably this needs reimplementation of all the neat things Qt already provides in the model/view classes. But as it copies data rather than referencing it, data exchange via signals between threads is possible as it is the recommended approach.

Which implementation should be chosen here? Regarding the threading, I fear approach 2. could still run into race conditions on read/write enabled items (but isn't that the case for magicgui widgets, too?)...

Again, #77 leaves this intentionally vague, so this might be the starting point.

I tried to provide some (ever so small) value in addition to my question, but if it still is too forumy-questiony, feel free to close this 😋

Thanks for having a look though!

@ndxmrb ndxmrb added the enhancement New feature or request label Aug 16, 2024
@tlambert03
Copy link
Member

hey @ndxmrb, #77 was essentially a pointer to a large amount of prior work and discussion that happened over in napari.

In napari, I did create QTreeView and QListView subclasses that were designed to do exactly this. Those were based on napari's evented model, before i extracted it into psygnal's variant. and #77 was just an acknowledgement that it would be nice to pull all of that work into superqt, and base it off of psygnal instead of the napari EventedModel.

I'm not going to lie, it gets rather hairy... but the end goal is indeed quite nice; python objects that automagically update themselves as you work with them. I would encourage you to start by looking at the napri code for QtListView, which would be a view onto the equivalent of a psygnal SelectableEventedList, and

@alisterburt took a stab at porting all of that over here in #105 ... so you could also start there and see what state it's in.

QTable and QTree would be a bit harder, since there's no direct model backing either of those quite like the SelectableEventedList is to a ListView, so additional decisions would need to be made.

I'm afraid i can't personally offer to work on this at the moment, but if you do dig deeper and have any questions as you go, feel free to keep asking them here

@ndxmrb
Copy link
Author

ndxmrb commented Aug 19, 2024

Hey @tlambert03,
thanks for the quick answer and the pointers.

I had a look at napari prior to my post here. I tried to use it as a role model, but got stuck (for now)... I also looked at the Issues over there, because I was hoping to find a road map for migrating to psygnal SelectableEventedList and friends. 😋 Is that the plan?

It's nice that the ground work for a list model has already been done. Do you remember why you didn't pull after the test was added? @alisterburt mentions a yet to be developed superqt QtListView in the docstrings. Why do we need this/what does it do? I thought by defining the model we achieve easy use of preexisting Qt views.

As for the additional decisions you mentioned: For tables and trees the main difference is, that conceptionally the ListModel is just a 1-to-1 mapper of the existing structure, whereas for tables and trees you essentially define a view model instead. E.g. you provide the columns you want to see [and the nested container path]. Other frameworks do this too and have different approaches. For instance, the close-to-pydantic FastUI does this with an extra DisplayLookup class. But it's difficult for me to gather all pros and cons for all these solutions.

Coming from EventedModels, a decision on what to show could be made in the ListModel as well though. Or would you rather do this by using delegates on the view?

@tlambert03
Copy link
Member

I also looked at the Issues over there, because I was hoping to find a road map for migrating to psygnal SelectableEventedList and friends. 😋 Is that the plan?

I can't speak to napari's plans for migration, I don't work on it anymore. I suspect there's no active push towards that direction at the moment though.

Do you remember why you didn't pull after the test was added?

i don't remember the exact reason at this point. my guess is that the tests were limited? but I really can't remember. If have the time, and you checkout that branch and can confirm that it does something (anything) that you would like to have merged into main. Then I'm not opposed to merging it in an experimental capacity.

mentions a yet to be developed superqt QtListView in the docstrings. Why do we need this/what does it do? I thought by defining the model we achieve easy use of preexisting Qt views.

I think his words were "first step towards a psygnal backed QtListView in superqt". So, I'm guessing he mostly means what you mean too (that the model is the main step). However, I'll note that in napari, it looks like I did create a QtView mixin that was designed specifically for our evented models. An example of using that mixin is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants