Skip to content

fix: fix signal connection between GUI and guiclass instance #693

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

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

Conversation

hanjinliu
Copy link
Collaborator

Fixes #690

This PR implements TwoWaySetAttr class for synchronization between widget and guiclass to safely handle the following problem.

The tricky part was that how many times signals will be emitted depends on the implementation of how values are set to the magicgui widget and to the backend widget.

Because Qt spin box widget does not emit Qt signal when the same value was set, the signal chain is like this:

  1. obj.gui.x.value changed
  2. emit value-changed signal (Qt)
  3. emit value-changed signal (psygnal)
  4. update obj.x
  5. emit obj.events.x
  6. update obj.gui.x.value
  7. does not emit value-changed signal (Qt) because same value was set

However, because the signal emissions of ListEdit is handled on the magicgui side, the signal chain becomes like this:

  1. obj.gui.x.value changed ← first time
  2. emit value-changed signal (psygnal)
  3. update obj.x
  4. emit obj.events.x
  5. update obj.gui.x.value
  6. obj.gui.x.value changed ← second time
  7. emit value-changed signal (psygnal)
  8. update obj.x
  9. does not emit obj.events.x because same value was set, thanks to the implementation of psygnal.evented

(We can consider 6. as a bug of ListEdit: it emits signal when the same value was set. In my opinion, this implementation should be left as is because checking values such as list[np.ndarray] is time consuming ... but let's leave this in other issues if we need more discussion)

Even if we carefully implement all the set_value methods, we may still have problems if a set-value method of some GUI library emits signal regardless of whether the old and new values are the same. The safest solution I think is to connect callbacks that can stop recursive two-way emission by itself.

Copy link

codecov bot commented Mar 9, 2025

Codecov Report

Attention: Patch coverage is 95.65217% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.75%. Comparing base (69c2808) to head (e68054e).

Files with missing lines Patch % Lines
src/magicgui/schema/_guiclass.py 95.12% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #693      +/-   ##
==========================================
- Coverage   88.85%   88.75%   -0.10%     
==========================================
  Files          40       40              
  Lines        4809     4847      +38     
==========================================
+ Hits         4273     4302      +29     
- Misses        536      545       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

IOT instruction with list in guiclass
1 participant