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

Don't require synced traits with VideoEditor #1621

Merged
merged 4 commits into from
May 4, 2021

Conversation

jwiggins
Copy link
Member

@corranwebster This is an implementation of what I believe you described as the intended use of Editor synced traits, applied to VideoEditor.

@jwiggins jwiggins force-pushed the refactor/vidjo-editor-traits branch from 0400dc0 to 3b5515a Compare April 28, 2021 13:12
@jwiggins
Copy link
Member Author

Care to take another look? I think I like the code this way (using ContextValue) a bit better but it's not quite right yet. Playing with the demo, it seems like the state isn't staying synchronized.

@corranwebster
Copy link
Contributor

This pretty much matches my version, so whichever we go with is fine.

@jwiggins jwiggins marked this pull request as ready for review April 28, 2021 16:15
@rahulporuri
Copy link
Contributor

CI failed with what looks like an intermittent failure. restarting job now.


test_base_url_changed (traitsui.tests.editors.test_html_editor.TestHTMLEditor) ... Fatal Python error: Illegal instruction

Current thread 0x00007f2fe2867740 (most recent call first):
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/traitsui/testing/_gui.py", line 36 in process_cascade_events
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/traitsui/testing/tester/ui_tester.py", line 116 in create_ui
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/contextlib.py", line 88 in __exit__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/traitsui/tests/editors/test_html_editor.py", line 253 in test_base_url_changed
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/case.py", line 605 in run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/case.py", line 653 in __call__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 122 in run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 84 in __call__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 122 in run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 84 in __call__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 122 in run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 84 in __call__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 122 in run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/suite.py", line 84 in __call__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/runner.py", line 176 in run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/main.py", line 256 in runTests
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/main.py", line 95 in __init__
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/unittest/__main__.py", line 18 in <module>
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/coverage/execfile.py", line 183 in run_python_file
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/coverage/execfile.py", line 113 in run_python_module
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/coverage/cmdline.py", line 627 in do_run
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/coverage/cmdline.py", line 483 in command_line
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/coverage/cmdline.py", line 756 in main
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/site-packages/coverage/__main__.py", line 8 in <module>
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/runpy.py", line 85 in _run_code
  File "/home/runner/work/traitsui/traitsui/.edm/envs/traitsui-test-3.6-pyside2/lib/python3.6/runpy.py", line 193 in _run_module_as_main
Command '['edm', 'run', '-e', 'traitsui-test-3.6-pyside2', '--', 'python', '-X', 'faulthandler', '-W', 'default', '-m', 'coverage', 'run', '-p', '-m', 'unittest', 'discover', '-v', 'traitsui']' returned non-zero exit status 252.

@corranwebster
Copy link
Contributor

This is failing consistently on an unrelated test.

@jwiggins
Copy link
Member Author

What's needed to move forward here?

@corranwebster
Copy link
Contributor

I think confirmation from the internal users who originally raised the issue that this solves it would be good.

I also think the ETS team are dealing with some non-work related issues today.

I'm good with the current state and if it's urgent to get it merged I can approve.

@jwiggins
Copy link
Member Author

Not urgent. I'll reach out to the affected users to solicit their feedback.

@rahulporuri rahulporuri self-requested a review May 3, 2021 05:22
@mdsmarte
Copy link
Contributor

mdsmarte commented May 4, 2021

Tested in context of our project and verified working as expected. Thanks for making the fix!

@corranwebster corranwebster merged commit c5182c6 into master May 4, 2021
@corranwebster corranwebster deleted the refactor/vidjo-editor-traits branch May 4, 2021 09:25
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.

None yet

4 participants