Skip to content

Convert dynamic on_trait_change to observe part 2 #880

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

Merged
merged 30 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
6cb6a5c
updates for both wx and qt action_item.py
aaronayres35 Jan 25, 2021
348d9be
first attempt at updating ui/qt4/action/menu_manager.py (something is…
aaronayres35 Jan 25, 2021
cb8abb9
this fixes the segfault but could break other things. _manager object…
aaronayres35 Feb 3, 2021
11bcb9c
correct fix
aaronayres35 Feb 3, 2021
267cf0c
update wx menu_manager.py
aaronayres35 Feb 3, 2021
a861827
update wx table_viewer.py and grid.py
aaronayres35 Feb 3, 2021
c497ec0
fix wx grid and table_viewer handler signatures
aaronayres35 Feb 4, 2021
23228ea
fix handler signaturess for wx menu_manager
aaronayres35 Feb 4, 2021
8694cea
update wx trait_grid_model
aaronayres35 Feb 4, 2021
194c3fc
updates for wx grid.py
aaronayres35 Feb 4, 2021
68d9a9c
remove a few more observers in dispose which weren't removed previously
aaronayres35 Feb 4, 2021
fa38e0b
remove commented out code
aaronayres35 Feb 4, 2021
6316309
update wx preference_dialog.py
aaronayres35 Feb 4, 2021
a972587
update wx list_box.py
aaronayres35 Feb 4, 2021
e3434f1
update wx tool_bar_manager.py
aaronayres35 Feb 4, 2021
8fa8527
update wx tree.py
aaronayres35 Feb 4, 2021
021359e
update qt tool_bar_manager
aaronayres35 Feb 4, 2021
0151354
update toggle_view_visibility_action
aaronayres35 Feb 4, 2021
5c17aee
update abstract_command_stack_action.py
aaronayres35 Feb 4, 2021
a9c74b3
update listening_action.py (note had to be careful observers were act…
aaronayres35 Feb 4, 2021
77a6e6c
maybe this is the more correct thing to do?
aaronayres35 Feb 4, 2021
ca4ca3e
update i_field.py
aaronayres35 Feb 4, 2021
0c77a63
update i_combo_field.py
aaronayres35 Feb 4, 2021
8a4c0aa
update i_spin_field.py
aaronayres35 Feb 4, 2021
e9b92e1
update i_text_field.py
aaronayres35 Feb 4, 2021
8a663b3
Apply suggestions from code review
aaronayres35 Feb 5, 2021
4e45c99
remove unneeded conditionals
aaronayres35 Feb 5, 2021
e79805b
update section in docs
aaronayres35 Feb 5, 2021
c581b0b
more suggestions from code review
aaronayres35 Feb 5, 2021
c51318d
Apply suggestions from code review
aaronayres35 Feb 5, 2021
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
13 changes: 9 additions & 4 deletions docs/source/timer.rst
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,21 @@ EventTimer
Another common use case is that you want a Traits Event to be fired
periodically or at some future time. This permits many possible listeners
to be called from the same timer, and have them be turned on and turned off
dynamically, if desired. The :py:class:`~pyface.timer.timer.EventTimer`
provides this functionality via its
:py:class:`~pyface.timer.timer.EventTimer.timeout` event trait.
dynamically, if desired, via
`Traits' observe <https://docs.enthought.com/traits/traits_user_manual/notification.html>`_
(note that observe listeners are required to take a single event argument).
The :py:class:`~pyface.timer.timer.EventTimer` provides this functionality via
its :py:class:`~pyface.timer.timer.EventTimer.timeout` event trait.

.. code-block:: python

from pyface.timer.api import EventTimer

def print_time(event):
print("The time is {}".format(datetime.datetime.now()))

timer = EventTimer(interval=0.5, expire=60)
timer.on_trait_change(print_time, 'timeout')
timer.observe(print_time, 'timeout')

timer.start()

Expand Down
34 changes: 19 additions & 15 deletions pyface/action/listening_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from pyface.action.action import Action
from traits.api import Any, Str
from traits.observation.api import trait

# Logging.
logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -55,11 +56,15 @@ def destroy(self):
"""

if self.object:
self.object.on_trait_change(
self._enabled_update, self.enabled_name, remove=True
self.object.observe(
self._enabled_update,
trait(self.enabled_name, optional=True),
remove=True
)
self.object.on_trait_change(
self._visible_update, self.visible_name, remove=True
self.object.observe(
self._visible_update,
trait(self.visible_name, optional=True),
Copy link
Contributor

@kitchoi kitchoi Feb 10, 2021

Choose a reason for hiding this comment

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

trait here assumes the first argument is a trait name (see doc), so an extended name like "child.is_visible" is going to fail because "child.is_visible" as a trait name is not defined on object. The optional=True may mask that lost behaviour?

remove=True
)

def perform(self, event=None):
Expand Down Expand Up @@ -102,32 +107,31 @@ def _enabled_name_changed(self, old, new):
obj = self.object
if obj is not None:
if old:
obj.on_trait_change(self._enabled_update, old, remove=True)
obj.observe(self._enabled_update, old, remove=True)
if new:
obj.on_trait_change(self._enabled_update, new)
obj.observe(self._enabled_update, new)
self._enabled_update()

def _visible_name_changed(self, old, new):
obj = self.object
if obj is not None:
if old:
obj.on_trait_change(self._visible_update, old, remove=True)
obj.observe(self._visible_update, old, remove=True)
if new:
obj.on_trait_change(self._visible_update, new)
obj.observe(self._visible_update, new)
self._visible_update()

def _object_changed(self, old, new):
for kind in ("enabled", "visible"):
method = getattr(self, "_%s_update" % kind)
name = getattr(self, "%s_name" % kind)
if name:
if old:
old.on_trait_change(method, name, remove=True)
if new:
new.on_trait_change(method, name)
if old:
old.observe(method, trait(name, optional=True), remove=True)
if new:
new.observe(method, trait(name, optional=True))
Comment on lines +128 to +131
Copy link
Contributor Author

Choose a reason for hiding this comment

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

commenting on this to draw attention to a reviewer. Previously the observer was never getting hooked up if name was None. This caused pyface/action/tests/test_listening_action/TestListeningAction/test_destroy to fail because we ultimately tried to unhook it in the destroy call.
The previous commit fixed this issue as well, but this seemed like the more correct way to go about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not entirely sure under what circumstances name is None will be True here. Ohh. Wait. Never mind. if name will be False if name is an empty string - which is possible.

method()

def _enabled_update(self):
def _enabled_update(self, event=None):
if self.enabled_name:
if self.object:
self.enabled = bool(
Expand All @@ -138,7 +142,7 @@ def _enabled_update(self):
else:
self.enabled = bool(self.object)

def _visible_update(self):
def _visible_update(self, event=None):
if self.visible_name:
if self.object:
self.visible = bool(
Expand Down
10 changes: 5 additions & 5 deletions pyface/fields/i_combo_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ def _initialize_control(self):
def _add_event_listeners(self):
""" Set up toolkit-specific bindings for events """
super(MComboField, self)._add_event_listeners()
self.on_trait_change(
self._values_updated, "values[],formatter", dispatch="ui"
self.observe(
self._values_updated, "values.items,formatter", dispatch="ui"
)
if self.control is not None:
self._observe_control_value()
Expand All @@ -79,9 +79,9 @@ def _remove_event_listeners(self):
""" Remove toolkit-specific bindings for events """
if self.control is not None:
self._observe_control_value(remove=True)
self.on_trait_change(
self.observe(
self._values_updated,
"values[],formatter",
"values.items,formatter",
dispatch="ui",
remove=True,
)
Expand All @@ -99,6 +99,6 @@ def _set_control_values(self, values):

# Trait change handlers --------------------------------------------------

def _values_updated(self):
def _values_updated(self, event):
if self.control is not None:
self._set_control_values(self.values)
25 changes: 14 additions & 11 deletions pyface/fields/i_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ class MField(HasTraits):
def _add_event_listeners(self):
""" Set up toolkit-specific bindings for events """
super(MField, self)._add_event_listeners()
self.on_trait_change(self._value_updated, "value", dispatch="ui")
self.on_trait_change(self._tooltip_updated, "tooltip", dispatch="ui")
self.on_trait_change(
self.observe(self._value_updated, "value", dispatch="ui")
self.observe(self._tooltip_updated, "tooltip", dispatch="ui")
self.observe(
self._context_menu_updated, "context_menu", dispatch="ui"
)
if self.control is not None and self.context_menu is not None:
Expand All @@ -70,13 +70,13 @@ def _remove_event_listeners(self):
""" Remove toolkit-specific bindings for events """
if self.control is not None and self.context_menu is not None:
self._observe_control_context_menu(remove=True)
self.on_trait_change(
self.observe(
self._value_updated, "value", dispatch="ui", remove=True
)
self.on_trait_change(
self.observe(
self._tooltip_updated, "tooltip", dispatch="ui", remove=True
)
self.on_trait_change(
self.observe(
self._context_menu_updated,
"context_menu",
dispatch="ui",
Expand Down Expand Up @@ -160,17 +160,20 @@ def _handle_control_context_menu(self, event):

# Trait change handlers -------------------------------------------------

def _value_updated(self, value):
def _value_updated(self, event):
value = event.new
if self.control is not None:
self._set_control_value(value)

def _tooltip_updated(self, tooltip):
def _tooltip_updated(self, event):
tooltip = event.new
if self.control is not None:
self._set_control_tooltip(tooltip)

def _context_menu_updated(self, old, new):
def _context_menu_updated(self, event):

if self.control is not None:
if new is None:
if event.new is None:
self._observe_control_context_menu(remove=True)
if old is None:
if event.old is None:
self._observe_control_context_menu()
6 changes: 3 additions & 3 deletions pyface/fields/i_spin_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,15 @@ def _initialize_control(self):
def _add_event_listeners(self):
""" Set up toolkit-specific bindings for events """
super(MSpinField, self)._add_event_listeners()
self.on_trait_change(self._bounds_updated, "bounds", dispatch="ui")
self.observe(self._bounds_updated, "bounds", dispatch="ui")
if self.control is not None:
self._observe_control_value()

def _remove_event_listeners(self):
""" Remove toolkit-specific bindings for events """
if self.control is not None:
self._observe_control_value(remove=True)
self.on_trait_change(
self.observe(
self._bounds_updated, "bounds", dispatch="ui", remove=True
)
super(MSpinField, self)._remove_event_listeners()
Expand Down Expand Up @@ -127,6 +127,6 @@ def _value_default(self):

# Trait change handlers --------------------------------------------------

def _bounds_updated(self):
def _bounds_updated(self, event):
if self.control is not None:
self._set_control_bounds(self.bounds)
32 changes: 13 additions & 19 deletions pyface/fields/i_text_field.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,10 @@ def _initialize_control(self):
def _add_event_listeners(self):
""" Set up toolkit-specific bindings for events """
super(MTextField, self)._add_event_listeners()
self.on_trait_change(
self._update_text_updated, "update_text", dispatch="ui"
)
self.on_trait_change(
self._placeholder_updated, "placeholder", dispatch="ui"
)
self.on_trait_change(self._echo_updated, "echo", dispatch="ui")
self.on_trait_change(
self._read_only_updated, "read_only", dispatch="ui"
)
self.observe(self._update_text_updated, "update_text", dispatch="ui")
self.observe(self._placeholder_updated, "placeholder", dispatch="ui")
self.observe(self._echo_updated, "echo", dispatch="ui")
self.observe(self._read_only_updated, "read_only", dispatch="ui")
if self.control is not None:
if self.update_text == "editing_finished":
self._observe_control_editing_finished()
Expand All @@ -91,22 +85,22 @@ def _remove_event_listeners(self):
self._observe_control_editing_finished(remove=True)
else:
self._observe_control_value(remove=True)
self.on_trait_change(
self.observe(
self._update_text_updated,
"update_text",
dispatch="ui",
remove=True,
)
self.on_trait_change(
self.observe(
self._placeholder_updated,
"placeholder",
dispatch="ui",
remove=True,
)
self.on_trait_change(
self.observe(
self._echo_updated, "echo", dispatch="ui", remove=True
)
self.on_trait_change(
self.observe(
self._read_only_updated, "read_only", dispatch="ui", remove=True
)
super(MTextField, self)._remove_event_listeners()
Expand Down Expand Up @@ -148,22 +142,22 @@ def _observe_control_editing_finished(self, remove=False):

# Trait change handlers --------------------------------------------------

def _placeholder_updated(self):
def _placeholder_updated(self, event):
if self.control is not None:
self._set_control_placeholder(self.placeholder)

def _echo_updated(self):
def _echo_updated(self, event):
if self.control is not None:
self._set_control_echo(self.echo)

def _read_only_updated(self):
def _read_only_updated(self, event):
if self.control is not None:
self._set_control_read_only(self.read_only)

def _update_text_updated(self, new):
def _update_text_updated(self, event):
""" Change how we listen to for updates to text value. """
if self.control is not None:
if new == "editing_finished":
if event.new == "editing_finished":
self._observe_control_value(remove=True)
self._observe_control_editing_finished()
else:
Expand Down
Loading