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.

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

Support changing images in ImageEditor for all IImage subclasses #1810

Merged
merged 7 commits into from
Feb 18, 2022

Conversation

corranwebster
Copy link
Contributor

@corranwebster corranwebster commented Feb 10, 2022

Currently code is doing a check for ImageResource. Includes smoke tests for expected functionality.

Checklist

  • Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)

@corranwebster corranwebster marked this pull request as ready for review February 10, 2022 13:04
Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

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

Generally changes LGTM, but I'm not sure why we're only testing on wx / I'm seeing test failures locally

image = Image()


@requires_toolkit([ToolkitName.wx])
Copy link
Contributor

Choose a reason for hiding this comment

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

why can't these be run on Qt?

Copy link
Contributor

Choose a reason for hiding this comment

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

The CI currently only runs on Qt.

Locally I ran python etstool.py install --toolkit=wx, edm shell -e traitsui-test-3.6-wx and python -m unittest traitsui.tests.editors.test_image_editor and I see the following test failures:

======================================================================
ERROR: test_image_editor_none (traitsui.tests.editors.test_image_editor.TestImageEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/tests/editors/test_image_editor.py", line 141, in test_image_editor_none
    with create_ui(obj1, dict(view=view)) as ui:
  File "/Users/aayres/.edm/envs/traitsui-test-3.6-wx/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/testing/tester/ui_tester.py", line 105, in create_ui
    ui = object.edit_traits(**ui_kwargs)
  File "/Users/aayres/.edm/envs/traitsui-test-3.6-wx/lib/python3.6/site-packages/traits/has_traits.py", line 1830, in edit_traits
    args,
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/view.py", line 450, in ui
    ui.ui(parent, kind)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/ui.py", line 235, in ui
    self.rebuild(self, parent)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/toolkit.py", line 118, in ui_live
    ui_live.ui_live(ui, parent)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_live.py", line 47, in ui_live
    _ui_dialog(ui, parent, BaseDialog.NONMODAL)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_live.py", line 81, in _ui_dialog
    BaseDialog.display_ui(ui, parent, style)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_base.py", line 61, in display_ui
    ui.owner.init(ui, parent, style)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_live.py", line 195, in init
    sw = panel(ui, window)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_panel.py", line 266, in panel
    panel, content[0], ui
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_panel.py", line 423, in fill_panel_for_group
    panel, group, ui, suppress_label, is_dock_window, create_panel
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_panel.py", line 577, in __init__
    self.add_items(content, panel, self.sizer)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/ui_panel.py", line 932, in add_items
    editor.prepare(item_panel)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/editor.py", line 247, in prepare
    self.init(parent)
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/wx/image_editor.py", line 44, in init
    self.control = ImageControl(parent, convert_bitmap(image), padding=0)
  File "/Users/aayres/.edm/envs/traitsui-test-3.6-wx/lib/python3.6/site-packages/pyface/ui_traits.py", line 92, in convert_bitmap
    return image.create_bitmap()
AttributeError: 'NoneType' object has no attribute 'create_bitmap'

======================================================================
ERROR: test_image_editor_resource (traitsui.tests.editors.test_image_editor.TestImageEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/tests/editors/test_image_editor.py", line 62, in test_image_editor_resource
    image=ImageResource(absolute_path=filename1)
TypeError: __init__() got an unexpected keyword argument 'absolute_path'

======================================================================
ERROR: test_image_editor_static (traitsui.tests.editors.test_image_editor.TestImageEditor)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/aayres/Desktop/all_ets/traitsui/traitsui/tests/editors/test_image_editor.py", line 51, in test_image_editor_static
    image=ImageResource(absolute_path=filename1),
TypeError: __init__() got an unexpected keyword argument 'absolute_path'

----------------------------------------------------------------------
Ran 5 tests in 1.216s

FAILED (errors=3)

running with pyside2 I see the same failures. In pyface.i_image_resource.MImageResource the __init__ method is overridden and doesn't call super.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should now be resolved. The absolute_path was me misremembering the API, not a bug in Pyface.

Copy link
Contributor

@aaronayres35 aaronayres35 left a comment

Choose a reason for hiding this comment

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

LGTM and all tests seem to be passing 🙌 (although Test with EDM / test-with-edm (macos-latest, pyside2) (pull_request) is being skipped on this PR and I'm not sure why 😕 - https://github.com/enthought/traitsui/runs/5230631738?check_suite_focus=true isn't very helpful)

@corranwebster
Copy link
Contributor Author

Re-ran the tests and all are passing, no skips.

@corranwebster corranwebster merged commit 9f3f17c into main Feb 18, 2022
@corranwebster corranwebster deleted the fix/image-editor-updates branch February 18, 2022 08:16
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.

2 participants