pyapp-kit / superqt

Missing widgets and components for Qt-python
https://pyapp-kit.github.io/superqt/
BSD 3-Clause "New" or "Revised" License
210 stars 38 forks source link

Searchable tree widget from a mapping #158

Closed andy-sweet closed 1 year ago

andy-sweet commented 1 year ago

This adds a searchable tree widget that can be initialized from a mapping. Mappings and iterables (other than strings) are traversed and populated in the tree. The tree has two columns.

The first column is either the key of a mapping or the enumerated index of an iterable, which can be searched based on regular expression. An item is shown if it, any of its ancestors, or any of its descendants match the expression. Ancestors are needed because tree items are only shown if all their ancestors are. Descendants are needed because if the user if look for a key that

The second column is either the str() value (for terminal types) or the type itself (e.g. dict) for non-terminal iterables.

I think this closes #12 , though I have not tried to reimplement OMETree using this yet.

Here's an example of using the tree on an OME-NGFF multiscales group .zattrs JSON file from Zenodo, which is easy to setup with just a json.load.

https://user-images.githubusercontent.com/2608297/232627769-9c74592d-8a4c-4d9f-af0c-e545e75e87b5.mp4

For XML, one would have to convert the element tree into a dictionary first (e.g. using [xmltodict].(https://github.com/martinblech/xmltodict)).

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.30 :tada:

Comparison is base (09c76a0) 85.48% compared to head (6af7f9f) 85.78%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #158 +/- ## ========================================== + Coverage 85.48% 85.78% +0.30% ========================================== Files 31 32 +1 Lines 2605 2660 +55 ========================================== + Hits 2227 2282 +55 Misses 378 378 ``` | [Impacted Files](https://codecov.io/gh/pyapp-kit/superqt/pull/158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit) | Coverage Δ | | |---|---|---| | [src/superqt/\_\_init\_\_.py](https://codecov.io/gh/pyapp-kit/superqt/pull/158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit#diff-c3JjL3N1cGVycXQvX19pbml0X18ucHk=) | `93.75% <100.00%> (ø)` | | | [src/superqt/selection/\_\_init\_\_.py](https://codecov.io/gh/pyapp-kit/superqt/pull/158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit#diff-c3JjL3N1cGVycXQvc2VsZWN0aW9uL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | | | [src/superqt/selection/\_searchable\_tree\_widget.py](https://codecov.io/gh/pyapp-kit/superqt/pull/158?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit#diff-c3JjL3N1cGVycXQvc2VsZWN0aW9uL19zZWFyY2hhYmxlX3RyZWVfd2lkZ2V0LnB5) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

tlambert03 commented 1 year ago

this is great. thanks @andy-sweet! I think the code is plenty readable and appropriately general for a first pass. If you can just add some tests I'm happy to get this is roughly as is

andy-sweet commented 1 year ago

I decided to make this a bit more general, refined the filtering behavior, and added some docs. I'll likely add some tests early next week and may do a little more polishing here. Will mark this ready for review when that's done.

tlambert03 commented 1 year ago

lemme know when ready... don't worry about the backend-specific test fails. trying to deal with that in another PR

andy-sweet commented 1 year ago

lemme know when ready... don't worry about the backend-specific test fails. trying to deal with that in another PR

Ready now. I changed a few things since you looked at the draft last.

Probably the biggest change is that I don't evaluate str() for mappings or iterable values and instead just show the type (e.g. dict). Without this change, large trees tend to be hard to read visually (i.e. lots of similar dictionary strings in the second column) and there are obvious performance concerns. Open to other ideas.

I also swapped the example for something a little more concise and JSON-like. I also wrote examples for more interesting JSON and XML, but didn't include them yet to avoid adding too much. Let me know if you'd like them.

andy-sweet commented 1 year ago

I also started wondering if the search term should affect the visibility of the items, or if we use it scroll to the first/best matching item. In all cases, there is a question about what to do with matches that are currently collapsed and therefore not visible to the user. Expanding is straightforward, but reverting to expansion/collapsed states after that is difficult.

goanpeca commented 1 year ago

I also started wondering if the search term should affect the visibility of the items, or if we use it scroll to the first/best matching item. In all cases, there is a question about what to do with matches that are current collapsed and therefore not visible to the user. Expanding is straightforward, but reverting to expansion/collapsed states after that is difficult

One could even get fancy and do fuzzy matching and highlight the found items 😅

andy-sweet commented 1 year ago

Another thought I had here is that this PR is contributing two things.

1. Searching/filtering a tree widget by the first column. 2. Creating and (wholesale) updating a tree widget from a mapping.

It could be worth splitting those two contributions, as there may be some desire for creating/updating a tree widget from nested non-mapping iterables too (where we only care about hierarchy and values).

tlambert03 commented 1 year ago

It could be worth splitting those two contributions, as there may be some desire for creating/updating a tree widget from nested non-mapping iterables too (where we only care about hierarchy and values).

happy to leave that splitting for when someone needs that functionality

tlambert03 commented 1 year ago

thanks!