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

DataView Image channel #683

Merged
merged 7 commits into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions examples/data_view/column_example.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
from functools import partial
from random import choice, randint

from traits.api import HasStrictTraits, Instance, Int, Str, List
from traits.api import Dict, HasStrictTraits, Instance, Int, Str, List

from pyface.api import ApplicationWindow, GUI
from pyface.api import ApplicationWindow, GUI, Image, ImageResource
from pyface.data_view.i_data_view_widget import IDataViewWidget
from pyface.data_view.data_view_widget import DataViewWidget
from pyface.data_view.value_types.api import IntValue, TextValue, no_value
Expand All @@ -25,6 +25,11 @@
AbstractRowInfo, ColumnDataModel, HasTraitsRowInfo
)

flags = {
'UK': ImageResource('gb.png'),
'USA': ImageResource('us.png'),
}


class Address(HasStrictTraits):

Expand All @@ -44,6 +49,17 @@ class Person(HasStrictTraits):
address = Instance(Address)


class CountryValue(TextValue):

flags = Dict(Str, Image, update_value_type=True)

def get_image(self, model, row, column):
value = model.get_value(row, column)
if value in self.flags:
return self.flags[value]
return None


row_info = HasTraitsRowInfo(
title='People',
value='name',
Expand Down Expand Up @@ -72,7 +88,9 @@ class Person(HasStrictTraits):
HasTraitsRowInfo(
title="Country",
value="address.country",
value_type=TextValue(),
value_type=CountryValue(
flags=flags,
),
),
],
),
Expand Down
Binary file added examples/data_view/images/gb.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions examples/data_view/images/image_LICENSE.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
The icons are mostly derived work from other icons. As such they are
licensed accordingly to the original license:

Project License File
------------------ --------------- -----------------------------------------
FamFamFam Flags Public Domain http://www.famfamfam.com/lab/icons/flags/

Unless stated in this file, icons are the work of Enthought, and are
released under a 3 clause BSD license.

Files and orginal authors:
----------------------------------------------------------------------------
examples/data_view/images:
gb.png | FamFamFam Flags
us.png | FamFamFam Flags
Binary file added examples/data_view/images/us.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
41 changes: 31 additions & 10 deletions pyface/data_view/abstract_value_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,11 @@ def set_text(self, model, row, column, text):
"""
raise DataViewSetError("Cannot set value.")

def has_tooltip(self, model, row, column):
""" Whether or not the value has a tooltip.
def has_image(self, model, row, column):
Copy link
Contributor

Choose a reason for hiding this comment

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

On naming... I notice that this is the same naming in the TableEditor / ListStrAdapter / etc. (https://github.com/enthought/traitsui/search?q=get_image&unscoped_q=get_image)
This image gets used with the DecorationRole, which is for displaying an icon or color (e.g. a color square next to the text).

Given the new data view is supposed to be more flexible, e.g. it needs to support alternative editors in the cell such as a spinbox or a combobox, we probably want to differentiate an icon that accompanies the rest of the cell from an image that fills up the entire cell. Since this is new code, could we name this has_icon and get_icon instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a distinction in most backends between an image and an icon - an icon is a collection of related images with different styles (ie. versions for enabled/disabled, highlighted, activated, etc.). This is very much not an icon in this sense, and Pyface has no object which corresponds to an icon (IImageResource tries to be both an image and an icon and fails at both). So these method as written are very much a "image"-based.

Let's leave this as-is for now, since this is currently the only way to display image data; we might revise when it comes time to implement custom editors/renderers and provide a separate channel for icon-like image collections.

Also note that Qt decorations are not limited to icons, and can potentially display thumbnails or even full images (as well as colors).

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. Just trying to say that as someone reading this method's docstring, it is not obvious that it is used as a decoration rather than an image filling up the whole cell. It was a bit of a surprise that the editable value is used alongside this image.

Qt documentation is very specific about what DecorationRole means:

The data to be rendered as a decoration in the form of an icon. (QColor, QIcon or QPixmap)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you have a large image and no text, the decoration will fill up the whole cell (or most of it - I think there may be a border, and you have to take into account how cell sizes are determined).

As someone writing an implementation of this method, you shouldn't care how this image is being displayed; you are providing some image data, and it is up to the view to determine what to do with it.

""" Whether or not the value has an image associated with it.

The default implementation returns True if ``get_tooltip``
returns a non-empty value.
The default implementation returns True if ``get_image``
returns a non-None value.

Parameters
----------
Expand All @@ -199,15 +199,15 @@ def has_tooltip(self, model, row, column):

Returns
-------
has_tooltip : bool
Whether or not the value has a textual representation.
has_image : bool
Whether or not the value has an image associated with it.
"""
return self.get_tooltip(model, row, column) != ""
return False

def get_tooltip(self, model, row, column):
""" The tooltip for the underlying value.
def get_image(self, model, row, column):
""" An image associated with the underlying value.

The default implementation returns an empty string.
The default implementation returns None.

Parameters
----------
Expand All @@ -220,6 +220,27 @@ def get_tooltip(self, model, row, column):

Returns
-------
image : IImageResource
The image associated with the underlying value.
"""
from pyface.image_resource import ImageResource
return ImageResource("image_not_found")

def has_tooltip(self, model, row, column):
""" Whether or not the value has a tooltip.

The default implementation returns True if ``get_tooltip``
returns a non-empty value.
has_tooltip : bool
Whether or not the value has a textual representation.
"""
return self.get_tooltip(model, row, column) != ""

def get_tooltip(self, model, row, column):
""" The tooltip for the underlying value.

The default implementation returns an empty string.

tooltip : str
The textual representation of the underlying value.
"""
Expand Down
10 changes: 10 additions & 0 deletions pyface/data_view/tests/test_abstract_value_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ def test_set_text(self):
with self.assertRaises(DataViewSetError):
value_type.set_text(self.model, [0], [0], "2.0")

def test_has_image(self):
value_type = ValueType()
result = value_type.has_image(self.model, [0], [0])
self.assertFalse(result)

def test_get_image(self):
value_type = ValueType()
result = value_type.get_image(self.model, [0], [0])
self.assertEqual(result.name, "image_not_found")

def test_parameter_update(self):
value_type = ValueType()
with self.assertTraitChanges(value_type, 'updated', count=1):
Expand Down
12 changes: 12 additions & 0 deletions pyface/data_view/value_types/constant_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from traits.api import Str

from pyface.data_view.abstract_value_type import AbstractValueType
from pyface.ui_traits import Image


class ConstantValue(AbstractValueType):
Expand All @@ -24,6 +25,9 @@ class ConstantValue(AbstractValueType):
#: The text value to display.
text = Str(update_value_type=True)

#: The image value to display.
image = Image(update_value_type=True)

#: The tooltip value to display.
tooltip = Str(update_value_type=True)

Expand All @@ -33,5 +37,13 @@ def has_editor_value(self, model, row, column):
def get_text(self, model, row, column):
return self.text

def has_image(self, model, row, column):
return self.image is not None

def get_image(self, model, row, column):
if self.image is not None:
return self.image
return super().get_image(model, row, column)

def get_tooltip(self, model, row, column):
return self.tooltip
3 changes: 3 additions & 0 deletions pyface/data_view/value_types/no_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ def has_editor_value(self, model, row, column):
def has_text(self, model, row, column):
return False

def has_image(self, model, row, column):
return False

def has_tooltip(self, model, row, column):
return False

Expand Down
29 changes: 29 additions & 0 deletions pyface/data_view/value_types/tests/test_constant_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from traits.testing.unittest_tools import UnittestTools

from pyface.data_view.value_types.constant_value import ConstantValue
from pyface.image_resource import ImageResource


class TestConstantValue(UnittestTools, TestCase):
Expand Down Expand Up @@ -51,6 +52,34 @@ def test_text_changed(self):
value_type.text = 'something'
self.assertEqual(value_type.text, 'something')

def test_has_image(self):
value_type = ConstantValue()
self.assertFalse(value_type.has_image(self.model, [0], [0]))

def test_has_image_true(self):
value_type = ConstantValue(image="question")
self.assertTrue(value_type.has_image(self.model, [0], [0]))

def test_get_image(self):
image = ImageResource("question")
value_type = ConstantValue(image=image)
self.assertEqual(
value_type.get_image(self.model, [0], [0]),
image
)

def test_get_image_none(self):
value_type = ConstantValue()
image = value_type.get_image(self.model, [0], [0])
self.assertEqual(image.name, "image_not_found")

def test_image_changed(self):
value_type = ConstantValue()
image = ImageResource("question")
with self.assertTraitChanges(value_type, 'updated'):
value_type.image = image
self.assertEqual(value_type.image, image)

def test_has_tooltip(self):
value_type = ConstantValue()
self.assertFalse(value_type.has_tooltip(self.model, [0], [0]))
Expand Down
4 changes: 4 additions & 0 deletions pyface/data_view/value_types/tests/test_no_value.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ def test_has_text(self):
value_type = NoValue()
self.assertFalse(value_type.has_text(self.model, [0], [0]))

def test_has_image(self):
value_type = NoValue()
self.assertFalse(value_type.has_image(self.model, [0], [0]))

def test_has_tooltip(self):
value_type = NoValue()
self.assertFalse(value_type.has_tooltip(self.model, [0], [0]))
6 changes: 6 additions & 0 deletions pyface/ui/qt4/data_view/data_view_item_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import logging

from pyface.i_image_resource import IImageResource
from pyface.qt import is_qt5
from pyface.qt.QtCore import QAbstractItemModel, QModelIndex, Qt
from pyface.data_view.index_manager import Root
Expand Down Expand Up @@ -148,6 +149,11 @@ def data(self, index, role=Qt.DisplayRole):
elif role == Qt.EditRole:
if value_type.has_editor_value(self.model, row, column):
return value_type.get_editor_value(self.model, row, column)
elif role == Qt.DecorationRole:
if value_type.has_image(self.model, row, column):
image = value_type.get_image(self.model, row, column)
if isinstance(image, IImageResource):
return image.create_image()
Copy link
Contributor

Choose a reason for hiding this comment

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

Observation, not a request change: Here to more untested code, but this entire module is untested and supposed to be rewritten.
I don't see an issue opened for rewriting this and getting it covered with tests, did I miss it or should I open it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a lot of things that don't have issues yet on the dataview roadmap. This is one of them.

elif role == Qt.ToolTipRole:
if value_type.has_tooltip(self.model, row, column):
return value_type.get_tooltip(self.model, row, column)
Expand Down