Skip to content

Conversation

@jwiggins
Copy link
Member

Drive-by while looking into RangeEditor strangeness.

try:
value = str(self.control.text())
value = eval(value)
value = literal_eval(value)
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not 100% sure about this one because I don't have much experience with the editor.

@jwiggins
Copy link
Member Author

I don't understand this failure. It doesn't seem related to the changes.

======================================================================
FAIL: test_qt_process_events_process_all (traitsui.testing.tests.test_gui.TestProcessEventsRepeated)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\traitsui\traitsui\.edm\envs\traitsui-test-3.6-pyside2\lib\site-packages\traitsui\testing\tests\test_gui.py", line 152, in test_qt_process_events_process_all
    self.assertEqual(n_left_behind_events, 0, msg)
AssertionError: 8 != 0 : Expected 10 events processed on the objects and zero events left on the queue after running process_cascade_events. Found 2 processed with 8 left behind.

@corranwebster
Copy link
Contributor

corranwebster commented Dec 21, 2022

Failure looks to be unrelated, unless the change is causing tests to not clean up after themselves properly.

I'll open an issue.

Edit: #1977

@corranwebster
Copy link
Contributor

Sat down and had a more thorough look at this and I think that literal_eval isn't the right thing for a bunch of these since many editors have an evaluate trait or method - eg. BaseRangeEditor provides this so at least the RangeEditor examples should probably replace eval with self.evaluate.

That feels like the right solution to propagate for most of these. However a sane default in some of these cases may be literal_eval and doing this in some cases may be an API change.

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