Move value mapping from factory to individual editors (#848)

* Move value mapping from factory to individual editors * Remove factory mapping references from image enum editor * Remove unused wx helper function * Address review comments * Remove test FIXMEs * Use instantiated tookit
enthought · Jul 8, 2020 · 592d028 · 592d028
1 parent dcd18ca
commit 592d028
Show file tree

Hide file tree

Showing 11 changed files with 98 additions and 222 deletions.
diff --git a/traitsui/editor_factory.py b/traitsui/editor_factory.py
@@ -309,15 +309,3 @@ class EditorWithListFactory(EditorFactory):
 
  #: Name of the trait on 'object' containing the enumeration data
  name = Str()
-
- #: Fired when the **values** trait has been updated:
- values_modified = Event()
-
- def _values_changed(self):
- """ Recomputes the mappings whenever the **values** trait is changed.
- """
- self._names, self._mapping, self._inverse_mapping = enum_values_changed(
- self.values, strfunc=self.string_value
- )
-
- self.values_modified = True
diff --git a/traitsui/qt4/enum_editor.py b/traitsui/qt4/enum_editor.py
@@ -59,7 +59,8 @@ class BaseEditor(Editor):
  # -------------------------------------------------------------------------
 
  def values_changed(self):
- """ Recomputes the cached data based on the underlying enumeration model.
+ """ Recomputes the cached data based on the underlying enumeration model
+ or the values of the factory.
  """
  self._names, self._mapping, self._inverse_mapping = enum_values_changed(
  self._value(), self.string_value
@@ -91,8 +92,10 @@ def init(self, parent):
  self._values_changed, " " + self._name, dispatch="ui"
  )
  else:
+ self._value = lambda: self.factory.values
+ self.values_changed()
  factory.on_trait_change(
- self.rebuild_editor, "values_modified", dispatch="ui"
+ self._values_changed, "values", dispatch="ui"
  )
 
  def dispose(self):
@@ -104,7 +107,7 @@ def dispose(self):
  )
  else:
  self.factory.on_trait_change(
- self.rebuild_editor, "values_modified", remove=True
+ self._values_changed, "values", remove=True
  )
 
  super(BaseEditor, self).dispose()
@@ -118,31 +121,23 @@ def dispose(self):
  def _get_names(self):
  """ Gets the current set of enumeration names.
  """
- if self._object is None:
- return self.factory._names
-
  return self._names
 
  def _get_mapping(self):
  """ Gets the current mapping.
  """
- if self._object is None:
- return self.factory._mapping
-
  return self._mapping
 
  def _get_inverse_mapping(self):
  """ Gets the current inverse mapping.
  """
- if self._object is None:
- return self.factory._inverse_mapping
-
  return self._inverse_mapping
 
  # Trait change handlers --------------------------------------------------
 
  def _values_changed(self):
- """ Handles the underlying object model's enumeration set being changed.
+ """ Handles the underlying object model's enumeration set or factory's
+ values being changed.
  """
  self.values_changed()
  self.rebuild_editor()

diff --git a/traitsui/qt4/image_enum_editor.py b/traitsui/qt4/image_enum_editor.py
@@ -57,6 +57,8 @@ def init(self, parent):
  """ Finishes initializing the editor by creating the underlying toolkit
  widget.
  """
+ super(ReadonlyEditor, self).init(parent)
+
  self.control = QtGui.QLabel()
  self.control.setPixmap(self.get_pixmap(self.str_value))
  self.set_tooltip()
@@ -67,6 +69,12 @@ def update_editor(self):
  """
  self.control.setPixmap(self.get_pixmap(self.str_value))
 
+ def rebuild_editor(self):
+ """ Rebuilds the contents of the editor whenever the original factory
+ object's **values** trait changes.
+ """
+ pass
+
 
 class SimpleEditor(BaseEditor, SimpleEnumEditor):
  """ Simple style of image enumeration editor, which displays a combo box.

diff --git a/traitsui/qt4/set_editor.py b/traitsui/qt4/set_editor.py
@@ -76,8 +76,10 @@ def init(self, parent):
  self._values_changed, self._name, dispatch="ui"
  )
  else:
+ self._value = lambda: self.factory.values
+ self.values_changed()
  factory.on_trait_change(
- self.update_editor, "values_modified", dispatch="ui"
+ self._values_changed, "values", dispatch="ui"
  )
 
  blayout = QtGui.QVBoxLayout()
@@ -120,25 +122,16 @@ def init(self, parent):
  def _get_names(self):
  """ Gets the current set of enumeration names.
  """
- if self._object is None:
- return self.factory._names
-
  return self._names
 
  def _get_mapping(self):
  """ Gets the current mapping.
  """
- if self._object is None:
- return self.factory._mapping
-
  return self._mapping
 
  def _get_inverse_mapping(self):
  """ Gets the current inverse mapping.
  """
- if self._object is None:
- return self.factory._inverse_mapping
-
  return self._inverse_mapping
 
  def _create_listbox(self, col, handler1, handler2, title):
@@ -171,14 +164,16 @@ def _create_button(self, label, layout, handler):
  return button
 
  def values_changed(self):
- """ Recomputes the cached data based on the underlying enumeration model.
+ """ Recomputes the cached data based on the underlying enumeration model
+ or the values of the factory.
  """
  self._names, self._mapping, self._inverse_mapping = enum_values_changed(
  self._value(), self.string_value
  )
 
  def _values_changed(self):
- """ Handles the underlying object model's enumeration set being changed.
+ """ Handles the underlying object model's enumeration set or factory's
+ values being changed.
  """
  self.values_changed()
  self.update_editor()
@@ -260,7 +255,7 @@ def dispose(self):
  )
  else:
  self.factory.on_trait_change(
- self.update_editor, "values_modified", remove=True
+ self._values_changed, "values", remove=True
  )
 
  self.context_object.on_trait_change(

diff --git a/traitsui/tests/editors/test_enum_editor.py b/traitsui/tests/editors/test_enum_editor.py
@@ -196,17 +196,10 @@ class IntEnumModel(HasTraits):
  with store_exceptions_on_all_threads():
  editor = self.setup_ui(IntEnumModel(), formatted_view)
 
- # FIXME issue enthought/traitsui#782
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["FALSE", "TRUE"])
- self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
- )
- self.assertEqual(editor.names, ["0", "1"])
- self.assertEqual(editor.mapping, {"0": 0, "1": 1})
+ self.assertEqual(editor.names, ["FALSE", "TRUE"])
+ self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
  self.assertEqual(
- editor.inverse_mapping, {0: "0", 1: "1"}
+ editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
  )
 
  enum_editor_factory.values = [1, 0]
@@ -238,47 +231,19 @@ class IntEnumModel(HasTraits):
  with store_exceptions_on_all_threads():
  editor = self.setup_ui(model, formatted_view)
 
- # FIXME issue enthought/traitsui#835
- if is_current_backend_wx():
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["FALSE", "TRUE"])
- self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
- )
- self.assertEqual(editor.names, ["0", "1"])
- self.assertEqual(editor.mapping, {"0": 0, "1": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "0", 1: "1"}
- )
- else:
- self.assertEqual(editor.names, ["FALSE", "TRUE"])
- self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
- )
+ self.assertEqual(editor.names, ["FALSE", "TRUE"])
+ self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
+ self.assertEqual(
+ editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
+ )
 
  model.possible_values = [1, 0]
 
- # FIXME issue enthought/traitsui#835
- if is_current_backend_wx():
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["TRUE", "FALSE"])
- self.assertEqual(editor.mapping, {"TRUE": 1, "FALSE": 0})
- self.assertEqual(
- editor.inverse_mapping, {1: "TRUE", 0: "FALSE"}
- )
- self.assertEqual(editor.names, ["1", "0"])
- self.assertEqual(editor.mapping, {"1": 1, "0": 0})
- self.assertEqual(
- editor.inverse_mapping, {1: "1", 0: "0"}
- )
- else:
- self.assertEqual(editor.names, ["TRUE", "FALSE"])
- self.assertEqual(editor.mapping, {"TRUE": 1, "FALSE": 0})
- self.assertEqual(
- editor.inverse_mapping, {1: "TRUE", 0: "FALSE"}
- )
+ self.assertEqual(editor.names, ["TRUE", "FALSE"])
+ self.assertEqual(editor.mapping, {"TRUE": 1, "FALSE": 0})
+ self.assertEqual(
+ editor.inverse_mapping, {1: "TRUE", 0: "FALSE"}
+ )
 
  def test_simple_editor_mapping_values(self):
  self.check_enum_mappings_value_change("simple", "radio")

diff --git a/traitsui/tests/editors/test_image_enum_editor.py b/traitsui/tests/editors/test_image_enum_editor.py
@@ -155,25 +155,14 @@ def check_enum_mappings_value_change(self, style):
  with store_exceptions_on_all_threads():
  editor = self.setup_ui(EnumModel(), formatted_view)
 
- # FIXME issue enthought/traitsui#782
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["TOP LEFT", "TOP RIGHT"])
- self.assertEqual(
- editor.mapping,
- {"TOP LEFT": "top left", "TOP RIGHT": "top right"}
- )
- self.assertEqual(
- editor.inverse_mapping,
- {"top left": "TOP LEFT", "top right": "TOP RIGHT"}
- )
- self.assertEqual(editor.names, ["top left", "top right"])
+ self.assertEqual(editor.names, ["TOP LEFT", "TOP RIGHT"])
  self.assertEqual(
  editor.mapping,
- {"top left": "top left", "top right": "top right"}
+ {"TOP LEFT": "top left", "TOP RIGHT": "top right"}
  )
  self.assertEqual(
  editor.inverse_mapping,
- {"top left": "top left", "top right": "top right"}
+ {"top left": "TOP LEFT", "top right": "TOP RIGHT"}
  )
 
  image_enum_editor_factory.values = ["top right", "top left"]

diff --git a/traitsui/tests/editors/test_set_editor.py b/traitsui/tests/editors/test_set_editor.py
@@ -170,17 +170,10 @@ class IntListModel(HasTraits):
  with store_exceptions_on_all_threads():
  editor = self.setup_ui(IntListModel(), formatted_view)
 
- # FIXME issue enthought/traitsui#782
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["FALSE", "TRUE"])
- self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
- )
- self.assertEqual(editor.names, ["0", "1"])
- self.assertEqual(editor.mapping, {"0": 0, "1": 1})
+ self.assertEqual(editor.names, ["FALSE", "TRUE"])
+ self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
  self.assertEqual(
- editor.inverse_mapping, {0: "0", 1: "1"}
+ editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
  )
 
  set_editor_factory.values = [1, 0]
@@ -211,47 +204,19 @@ class IntListModel(HasTraits):
  with store_exceptions_on_all_threads():
  editor = self.setup_ui(model, formatted_view)
 
- # FIXME issue enthought/traitsui#835
- if is_current_backend_wx():
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["FALSE", "TRUE"])
- self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
- )
- self.assertEqual(editor.names, ["0", "1"])
- self.assertEqual(editor.mapping, {"0": 0, "1": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "0", 1: "1"}
- )
- else:
- self.assertEqual(editor.names, ["FALSE", "TRUE"])
- self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
- self.assertEqual(
- editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
- )
+ self.assertEqual(editor.names, ["FALSE", "TRUE"])
+ self.assertEqual(editor.mapping, {"FALSE": 0, "TRUE": 1})
+ self.assertEqual(
+ editor.inverse_mapping, {0: "FALSE", 1: "TRUE"}
+ )
 
  model.possible_values = [1, 0]
 
- # FIXME issue enthought/traitsui#835
- if is_current_backend_wx():
- with self.assertRaises(AssertionError):
- self.assertEqual(editor.names, ["TRUE", "FALSE"])
- self.assertEqual(editor.mapping, {"TRUE": 1, "FALSE": 0})
- self.assertEqual(
- editor.inverse_mapping, {1: "TRUE", 0: "FALSE"}
- )
- self.assertEqual(editor.names, ["1", "0"])
- self.assertEqual(editor.mapping, {"1": 1, "0": 0})
- self.assertEqual(
- editor.inverse_mapping, {1: "1", 0: "0"}
- )
- else:
- self.assertEqual(editor.names, ["TRUE", "FALSE"])
- self.assertEqual(editor.mapping, {"TRUE": 1, "FALSE": 0})
- self.assertEqual(
- editor.inverse_mapping, {1: "TRUE", 0: "FALSE"}
- )
+ self.assertEqual(editor.names, ["TRUE", "FALSE"])
+ self.assertEqual(editor.mapping, {"TRUE": 1, "FALSE": 0})
+ self.assertEqual(
+ editor.inverse_mapping, {1: "TRUE", 0: "FALSE"}
+ )
 
 
 @skip_if_null