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

Port base item model for SelectableEventedList backed ListView from napari #105

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

alisterburt
Copy link

This PR ports the _BaseEventedItemModel, an adapter between the QAbstractItemModel and our SelectableEventedList, from napari to superqt. This is a first step towards a psygnal backed QtListView in superqt

Original code in napari: https://github.com/napari/napari/blob/main/napari/_qt/containers/_base_item_model.py

The code has been updated to work with the psygnal SelectableEventedList, requiring only minimal changes.

I didn't reimplement the process_event hook, I wasn't exactly sure when this would be needed and how it fit with psygnal/qt style events

@codecov
Copy link

codecov bot commented Aug 2, 2022

Codecov Report

Base: 85.30% // Head: 81.83% // Decreases project coverage by -3.47% ⚠️

Coverage data is based on head (60ccb84) compared to base (6ce87d4).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #105      +/-   ##
==========================================
- Coverage   85.30%   81.83%   -3.48%     
==========================================
  Files          31       34       +3     
  Lines        2607     2719     +112     
==========================================
+ Hits         2224     2225       +1     
- Misses        383      494     +111     
Impacted Files Coverage Δ
src/superqt/listview/__init__.py 0.00% <0.00%> (ø)
src/superqt/listview/_base_item_model.py 0.00% <0.00%> (ø)
src/superqt/listview/_list_model.py 0.00% <0.00%> (ø)
src/superqt/utils/_code_syntax_highlight.py 98.03% <0.00%> (+1.96%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tlambert03
Copy link
Member

hey @alisterburt ... man, I swear I commented on this before, but I guess not 😂
I would love to have this in here. if you're still interested, i think copying over the relevant tests from napari would be a great sanity check to see whether this is working. If you don't have time, I can probably pick it up

@alisterburt
Copy link
Author

Ello ello! Thanks for the ping and sorry for shelving this for so long, the curse of trying to contribute things I don't strictly need 😆

I've got a bit of time now so will move the tests over 🙂

@alisterburt
Copy link
Author

might be being stupid but the only tests I could see were the listmodel test so I ported that over and fixed a minor bug. I likely won't have much more time for this in the next few days so please feel free to push it over the line if you have momentum!

@tlambert03
Copy link
Member

thanks for what you did! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants