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

Jump to bottom

Replace dynamic uses of on_trait_change with observe part 2 #663

Merged
merged 18 commits into from
Mar 9, 2021
Merged

Replace dynamic uses of on_trait_change with observe part 2 #663

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
de4f35a
update enable/text_field.py
aaronayres35 Mar 2, 2021
7ee8432
update enable/drawing/drawing_canvas.py
aaronayres35 Mar 2, 2021
b2dc673
update enable/examples/demo/enable/compass_example.py
aaronayres35 Mar 2, 2021
c72be9e
update enable/examples/demo/enable/scrollbar_demo.py
aaronayres35 Mar 2, 2021
f346414
update enable/examples/demo/enable/slider_example.py
aaronayres35 Mar 2, 2021
901a321
update enable/examples/demo/enable/tools/button_tool.py
aaronayres35 Mar 2, 2021
dbd19b7
update qt and wx svg_button_editor.py
aaronayres35 Mar 2, 2021
078b521
update enable/trait_defs/ui/wx/enable_rgba_color_editor.py
aaronayres35 Mar 2, 2021
77d426d
update enable/trait_defs/ui/wx/rgba_color_editor.py
aaronayres35 Mar 2, 2021
cc3f76c
update enable/abstract_window.py
aaronayres35 Mar 2, 2021
d64e94f
remove old now incorrect comment
aaronayres35 Mar 8, 2021
361fa1d
rename to avoid _changed confusion
aaronayres35 Mar 8, 2021
9fbf726
add a prerviusly failing, now passing test
aaronayres35 Mar 9, 2021
5dc3f89
style comments copyright
aaronayres35 Mar 9, 2021
d014927
rename changed to updated
aaronayres35 Mar 9, 2021
61a06ae
rewrite a strange conditional check
aaronayres35 Mar 9, 2021
db9a6de
Revert "update qt and wx svg_button_editor.py"
aaronayres35 Mar 9, 2021
d8f0f13
check new is not None, not self.component
aaronayres35 Mar 9, 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
35 changes: 18 additions & 17 deletions enable/abstract_window.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ def __init__(self, **traits):

def _component_changed(self, old, new):
if old is not None:
old.on_trait_change(
old.observe(
self.component_bounds_changed, "bounds", remove=True
)
old.window = None
Expand All @@ -218,25 +218,26 @@ def _component_changed(self, old, new):
# If possible, size the new component according to the size of the
# toolkit control
size = self._get_control_size()
if (size is not None) and hasattr(self.component, "bounds"):
new.on_trait_change(self.component_bounds_changed, "bounds")
pix_scale = self.base_pixel_scale
if getattr(self.component, "fit_window", False):
self.component.outer_position = [0, 0]
self.component.outer_bounds = [
size[0] / pix_scale, size[1] / pix_scale
]
elif hasattr(self.component, "resizable"):
if "h" in self.component.resizable:
self.component.outer_x = 0
self.component.outer_width = size[0] / pix_scale
if "v" in self.component.resizable:
self.component.outer_y = 0
self.component.outer_height = size[1] / pix_scale
if hasattr(self.component, "bounds"):
jwiggins marked this conversation as resolved.
Show resolved Hide resolved
new.observe(self.component_bounds_changed, "bounds")
if size is not None:
pix_scale = self.base_pixel_scale
if getattr(self.component, "fit_window", False):
self.component.outer_position = [0, 0]
self.component.outer_bounds = [
size[0] / pix_scale, size[1] / pix_scale
]
elif hasattr(self.component, "resizable"):
if "h" in self.component.resizable:
self.component.outer_x = 0
self.component.outer_width = size[0] / pix_scale
if "v" in self.component.resizable:
self.component.outer_y = 0
self.component.outer_height = size[1] / pix_scale
self._update_region = None
self.redraw()

def component_bounds_changed(self, bounds):
def component_bounds_changed(self, event):
"""
Dynamic trait listener that handles our component changing its size;
bounds is a length-2 list of [width, height].
aaronayres35 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
12 changes: 4 additions & 8 deletions enable/drawing/drawing_canvas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,18 +159,14 @@ def add_button(self, *buttons):

def _canvas_changed(self, old, new):
if old:
old.on_trait_change(
self._canvas_bounds_changed, "bounds", remove=True
)
old.on_trait_change(
self._canvas_bounds_changed, "bounds_items", remove=True
old.observe(
self._canvas_bounds_changed, "bounds.items", remove=True
)

if new:
new.on_trait_change(self._canvas_bounds_changed, "bounds")
new.on_trait_change(self._canvas_bounds_changed, "bounds_items")
new.observe(self._canvas_bounds_changed, "bounds.items")

def _canvas_bounds_changed(self):
def _canvas_bounds_changed(self, event):
aaronayres35 marked this conversation as resolved.
Show resolved Hide resolved
self.width = self.canvas.width
self.y = self.canvas.height - self.height

Expand Down
4 changes: 2 additions & 2 deletions enable/examples/demo/enable/compass_example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ def _create_component(self):
container = OverlayContainer()
container.add(compass)

compass.on_trait_change(self._arrow_printer, "clicked")
compass.observe(self._arrow_printer, "clicked")
self.compass = compass
return container

def _arrow_printer(self):
def _arrow_printer(self, event):
print("Clicked:", self.compass.clicked)


Expand Down
13 changes: 6 additions & 7 deletions enable/examples/demo/enable/scrollbar_demo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _create_component(self):
range=(0, 100.0, 10.0, 1.0),
enabled=True,
)
vscroll.on_trait_change(self._update_vscroll, "scroll_position")
vscroll.observe(self._update_vscroll, "scroll_position")

hscroll = NativeScrollBar(
orientation="horizontal",
Expand All @@ -40,31 +40,30 @@ def _create_component(self):
range=(0, 100.0, 10.0, 1.0),
enabled=True,
)
hscroll.on_trait_change(self._update_hscroll, "scroll_position")
hscroll.observe(self._update_hscroll, "scroll_position")

container = Container(
bounds=[200, 200], border_visible=True, padding=15
)
container.add(label, hscroll, vscroll)
container.on_trait_change(self._update_layout, "bounds")
container.on_trait_change(self._update_layout, "bounds_items")
container.observe(self._update_layout, "bounds.items")

self.label = label
self.hscroll = hscroll
self.vscroll = vscroll
return container

def _update_hscroll(self):
def _update_hscroll(self, event):
text = self.label.text.split("\n")
text[0] = "h: " + str(self.hscroll.scroll_position)
self.label.text = "\n".join(text)

def _update_vscroll(self):
def _update_vscroll(self, event):
text = self.label.text.split("\n")
text[1] = "v: " + str(self.vscroll.scroll_position)
self.label.text = "\n".join(text)

def _update_layout(self):
def _update_layout(self, event):
self.vscroll._widget_moved = True
self.hscroll._widget_moved = True

Expand Down
4 changes: 2 additions & 2 deletions enable/examples/demo/enable/slider_example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ def _create_component(self):
container = OverlayContainer()
container.add(slider)

slider.on_trait_change(self.val_changed, "value")
slider.observe(self.val_changed, "value")
self.slider = slider
return container

def val_changed(self):
def val_changed(self, event):
print(self.slider.value)


Expand Down
11 changes: 6 additions & 5 deletions enable/examples/demo/enable/tools/button_tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class SelectableBox(Box):

selected_color = ColorTrait("green")

def select(self, selected):
def select(self, event):
selected = event.new
self.selected = selected

def _selected_changed(self):
Expand Down Expand Up @@ -68,7 +69,7 @@ class MyFrame(DemoFrame):

"""

def button_clicked(self):
def button_clicked(self, event):
print("clicked")

# -------------------------------------------------------------------------
Expand All @@ -86,8 +87,8 @@ def _create_component(self):
push_button_box.tools.append(push_button_tool)

# print when box clicked, change color when button down
push_button_tool.on_trait_change(self.button_clicked, "clicked")
push_button_tool.on_trait_change(push_button_box.select, "down")
push_button_tool.observe(self.button_clicked, "clicked")
push_button_tool.observe(push_button_box.select, "down")

# another box for a toggle button
toggle_box = SelectableBox(
Expand All @@ -102,7 +103,7 @@ def _create_component(self):
toggle_box.tools.append(toggle_button_tool)

# change color when checked down
toggle_button_tool.on_trait_change(toggle_box.select, "checked")
toggle_button_tool.observe(toggle_box.select, "checked")

container = Container(bounds=[600, 600])
container.add(push_button_box)
Expand Down
4 changes: 2 additions & 2 deletions enable/savage/trait_defs/ui/qt4/svg_button_editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def prepare(self, parent):
"""
name = self.extended_name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BEWARE: extended_name

Copy link
Member

@jwiggins jwiggins Mar 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Meaning what, exactly? We can't be sure that name won't break when passed to observe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, a quick search of the code base for extended_name only gives this result here and the corresponding same thing in the wx file. Any downstream users that set this trait may run into problems if the extended name they are passing does not conform to the new mini language, and that is effectively out of our hands ...

A similar scenario was encountered in pyface and we mentioned it in the release announcement. I imagine the vast majority of users would not be affected (I dont even know how many users of this code we have in total). Nonetheless, it leaves a door open for potential breaks.

Another thing to worry about here though that I'm not sure what we would want to do for: The traitsUI code for Editor contains the code that would theoretically remove this listener (in either the dispose method or the _update_editor method itself. I also just realize I made a dumb mistake! The method being observed is _update_editor not update_editor which is defined in traitsUI. We are still in the process of updating traitsUI to observe, so that method will need its signature changed... I believe we will need that around first, before we can make this change as this code won't work currently. It must be uncovered in the test suite hence we don't see a failure here

if name != "None":
self.context_object.on_trait_change(
self.context_object.observe(
aaronayres35 marked this conversation as resolved.
Show resolved Hide resolved
self._update_editor, name, dispatch="ui"
)
self.init(parent)
Expand All @@ -105,7 +105,7 @@ def update_object(self):
"""
self.value = self.factory.value

def update_editor(self):
def update_editor(self, event):
""" Updates the editor when the object trait changes externally to the
editor.
"""
Expand Down
4 changes: 2 additions & 2 deletions enable/savage/trait_defs/ui/wx/svg_button_editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,13 @@ def prepare(self, parent):
"""
name = self.extended_name
if name != "None":
self.context_object.on_trait_change(
self.context_object.observe(
self._update_editor, name, dispatch="ui"
)
self.init(parent)
self._sync_values()

def update_editor(self):
def update_editor(self, event):
""" Updates the editor when the object trait changes externally to the
editor.
"""
Expand Down
9 changes: 6 additions & 3 deletions enable/text_field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,12 +460,15 @@ def _refresh_viewed_line(self, line):
def _acquire_focus(self, window):
self._draw_cursor = True
window.focus_owner = self
window.on_trait_change(self._focus_owner_changed, "focus_owner")
window.observe(self._focus_owner_changed, "focus_owner")
self.request_redraw()

def _focus_owner_changed(self, obj, name, old, new):
def _focus_owner_changed(self, event):
aaronayres35 marked this conversation as resolved.
Show resolved Hide resolved
obj = event.object
old = event.old
new = event.new
if old == self and new != self:
obj.on_trait_change(
obj.observe(
self._focus_owner_changed, "focus_owner", remove=True
)
self._draw_cursor = False
Expand Down
4 changes: 2 additions & 2 deletions enable/trait_defs/ui/wx/enable_rgba_color_editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ def init(self, parent):
self.control = window.control
self._picker = picker
self.control.SetSize((picker.min_width, picker.min_height))
picker.on_trait_change(self.popup_editor, "clicked", dispatch="ui")
picker.on_trait_change(self.update_object, "color", dispatch="ui")
picker.observe(self.popup_editor, "clicked", dispatch="ui")
picker.observe(self.update_object, "color", dispatch="ui")

# -------------------------------------------------------------------------
# Invokes the pop-up editor for an object trait:
Expand Down
2 changes: 1 addition & 1 deletion enable/trait_defs/ui/wx/rgba_color_editor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def init(self, parent):
self._swatch = window.component
self.control = window.control
self.control.SetSize((110, 20))
window.component.on_trait_change(
window.component.observe(
self.popup_editor, "left_up", dispatch="ui"
)

Expand Down