Skip to content

More _changed method updates to observe#963

Merged
aaronayres35 merged 15 commits into
masterfrom
more-_changed-method-updates
Jul 1, 2021
Merged

More _changed method updates to observe#963
aaronayres35 merged 15 commits into
masterfrom
more-_changed-method-updates

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 commented Jun 22, 2021

Continuation of #954, and therefor part of #899

This PR simply replaces some more _*_changed magic named static handlers with observe.

@aaronayres35
Copy link
Copy Markdown
Contributor Author

Going to remove the WIP label on this now. This is a completely arbitrary choice, but I figure keep PRs small

@aaronayres35 aaronayres35 requested a review from rahulporuri June 28, 2021 12:24
@aaronayres35 aaronayres35 changed the title [WIP] More _changed method updates to observe More _changed method updates to observe Jun 28, 2021
Comment thread pyface/ui/qt4/tasks/advanced_editor_area_pane.py Outdated
Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

The changes themselves LGTM but I have made a bunch of suggestions - mostly related to renaming the private observe handlers. I'm okay if the suggestions are ignored but I think the suggested renaming (or any changes proposed to the suggestions themselves) are a step in the right direction for the codebase.

Comment thread pyface/wizard/wizard_controller.py Outdated
Comment thread pyface/wizard/chained_wizard_controller.py Outdated
Comment thread pyface/wizard/chained_wizard_controller.py Outdated
Comment thread pyface/wizard/chained_wizard.py Outdated
Comment thread pyface/wizard/chained_wizard.py Outdated
Comment thread pyface/tree/node_tree.py Outdated
Comment thread pyface/tree/node_manager.py Outdated
Comment thread pyface/tasks/task_window.py Outdated
Comment thread pyface/dock/dock_window_feature.py Outdated
Comment thread pyface/dock/dock_window.py Outdated
aaronayres35 and others added 2 commits June 29, 2021 05:09
Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
@aaronayres35
Copy link
Copy Markdown
Contributor Author

CI is failing with:

[EXECUTING] /home/runner/work/pyface/pyface/.edm/envs/edm/bin/edm run -e pyface-test-3.6-wx -- python -Xfaulthandler -m coverage run -p -m unittest discover -v pyface
Shell 'sh' is not supported, things may not work properly (supported shells are bash, zsh)
pyface (unittest.loader._FailedTest) ... ERROR

======================================================================
ERROR: pyface (unittest.loader._FailedTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/runner/work/pyface/pyface/.edm/envs/pyface-test-3.6-wx/lib/python3.6/site-packages/pyface/__init__.py", line 61, in load_tests
    from pyface.toolkit import toolkit_object  # noqa: F401
  File "/home/runner/work/pyface/pyface/.edm/envs/pyface-test-3.6-wx/lib/python3.6/site-packages/pyface/toolkit.py", line 23, in <module>
    toolkit = toolkit_object = find_toolkit("pyface.toolkits")
  File "/home/runner/work/pyface/pyface/.edm/envs/pyface-test-3.6-wx/lib/python3.6/site-packages/pyface/base_toolkit.py", line 256, in find_toolkit
    return import_toolkit(ETSConfig.toolkit, entry_point)
  File "/home/runner/work/pyface/pyface/.edm/envs/pyface-test-3.6-wx/lib/python3.6/site-packages/pyface/base_toolkit.py", line 220, in import_toolkit
    raise RuntimeError(msg)
RuntimeError: No pyface.toolkits plugin could be loaded for wx

----------------------------------------------------------------------
Ran 1 test in 0.000s

FAILED (errors=1)
Command '['/home/runner/work/pyface/pyface/.edm/envs/edm/bin/edm', 'run', '-e', 'pyface-test-3.6-wx', '--', 'python', '-Xfaulthandler', '-m', 'coverage', 'run', '-p', '-m', 'unittest', 'discover', '-v', 'pyface']' returned non-zero exit status 1.
/usr/bin/bash /home/runner/work/_actions/GabrielBB/xvfb-action/v1/cleanup.sh
Killing the following xvfb processes: 2964
Error: The process '/usr/bin/xvfb-run' failed with exit code 1

@aaronayres35
Copy link
Copy Markdown
Contributor Author

suspect test failure is related GH-Actions being enabled.
#950 made its way through before hand (I could have sworn I remembered gh actions being run on that PR but I must be mistaken).

@aaronayres35
Copy link
Copy Markdown
Contributor Author

aaronayres35 commented Jun 29, 2021

Okay CI is closer, but now I'm seeing:

======================================================================
FAIL: test_focus (pyface.tests.test_widget.TestConcreteWidget)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\pyface\pyface\.edm\envs\pyface-test-3.6-pyside2\lib\site-packages\pyface\tests\test_widget.py", line 331, in test_focus
    self.assertFalse(self.widget.has_focus())
AssertionError: True is not false

----------------------------------------------------------------------
Ran 1098 tests in 78.060s

FAILED (failures=1, skipped=29)
Command '['D:\\a\\pyface\\pyface\\.edm\\envs\\edm\\Scripts\\edm.exe', 'run', '-e', 'pyface-test-3.6-pyside2', '--', 'python', '-Xfaulthandler', '-m', 'coverage', 'run', '-p', '-m', 'unittest', 'discover', '-v', 'pyface']' returned non-zero exit status 1.
Error: The process 'C:\Enthought\edm\edm.bat' failed with exit code 1

Unfortunately I believe these are all orthogonal to this PR...

@aaronayres35 aaronayres35 requested a review from rahulporuri June 29, 2021 15:19
@aaronayres35
Copy link
Copy Markdown
Contributor Author

@rahulporuri This PR still look good to go? Since your review I had to make a few CI related changes as GH Actions was enabled for the repo and CI started failing

Copy link
Copy Markdown
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

still LGTM

@aaronayres35 aaronayres35 merged commit a9cbd8f into master Jul 1, 2021
@aaronayres35 aaronayres35 deleted the more-_changed-method-updates branch July 1, 2021 11:42
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