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.

Sign up for GitHub

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

MAINT: add main thread check for ui dispatch, solve no ui failure #1740

Merged
merged 18 commits into from
Apr 24, 2023
Merged

MAINT: add main thread check for ui dispatch, solve no ui failure #1740

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
3050840
MAINT: add main thread check for ui dispatch, solve no ui failure
Apr 17, 2023
572bc99
Merge branch 'main' into fix_no_ui_failure
homosapien-lcy Apr 19, 2023
c8857fb
MAINT: add unittest for ui_patch change
Apr 19, 2023
16203ed
MAINT: add systemexit to make the error noticed in the main thread
Apr 19, 2023
2163e96
change a print to pass to make the message clean
Apr 19, 2023
ac0d2b2
NIT: fix formatting issues
Apr 19, 2023
f49cd27
add explanation of ui_dispatch behavior
Apr 19, 2023
f25afbc
fix inline emphasis
Apr 19, 2023
b929e6f
add observer action to test the observer is called
Apr 20, 2023
5219d61
STYLE: add space before loop
Apr 20, 2023
148f654
MAINT: remove system exit try except
Apr 20, 2023
d758a9d
DOC: change explanaton of observe
Apr 20, 2023
476daf4
MAINT: use more rigorous test for notifier
Apr 21, 2023
334845c
Merge branch 'main' into fix_no_ui_failure
homosapien-lcy Apr 24, 2023
927e04b
MAINT: add length test for eventlist (test_trait_notifiers)
Apr 24, 2023
504349b
Add documentation that covers the main thread path for ui dispatch
mdickinson Apr 24, 2023
8a37a0e
Remove misplaced documentation from the migration section
mdickinson Apr 24, 2023
a15e1f7
Style fixes
mdickinson Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion traits/trait_notifiers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def set_ui_handler(handler):


def ui_dispatch(handler, *args, **kw):
if threading.current_thread().ident == ui_thread:
if threading.current_thread().ident == threading.main_thread().ident:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the .ident here: you can just compare the thread objects directly.

Suggested change
if threading.current_thread().ident == threading.main_thread().ident:
if threading.current_thread() == threading.main_thread():

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. Are directly comparing threads vs comparing their ids completely equivalent?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, yes. The == check is essentially an is check (there's no __eq__ special method created), and there should be exactly one Thread object per OS thread.

handler(*args, **kw)
else:
ui_handler(handler, *args, **kw)
Expand Down