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

FIX: Replace unnecessary eval() calls with literal_eval() #1976

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jwiggins
Copy link
Member

Drive-by while looking into RangeEditor strangeness.

@@ -266,7 +267,7 @@ def update_object(self, event=None):
"""Handles the user changing the contents of the edit control."""
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