Skip to content

Color text based on background color when using _background_gradient() #21263

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

Merged
Merged
Show file tree
Hide file tree
Changes from 14 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
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ Reshaping
Other
^^^^^

-
- :meth: `~pandas.io.formats.style.Styler.background_gradient` now takes a ``text_color_threshold`` parameter to automatically lighten the text color based on the luminance of the background color. This improves readability with dark background colors without the need to limit the background colormap range. (:issue:`21258`, :issue:`21269`)
Copy link
Member

Choose a reason for hiding this comment

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

Can just reference the first issue here, since that is what you are closing anyway (second was a duplicate)

-
-

62 changes: 53 additions & 9 deletions pandas/io/formats/style.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,7 @@ def highlight_null(self, null_color='red'):
return self

def background_gradient(self, cmap='PuBu', low=0, high=0, axis=0,
subset=None):
subset=None, text_color_threshold=0.408):
"""
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add a Raises section for the docstring?

Color the background in a gradient according to
the data in each column (optionally row).
Expand All @@ -879,26 +879,39 @@ def background_gradient(self, cmap='PuBu', low=0, high=0, axis=0,
1 or 'columns' for columnwise, 0 or 'index' for rowwise
subset: IndexSlice
a valid slice for ``data`` to limit the style application to
text_color_threshold: float or int
Copy link
Contributor

Choose a reason for hiding this comment

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

Add , default 0.408.

And may a note as to why that's the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We chose 0.408 to stay consistent with the Seaborn implementation. Should I ask the Seaborn author for the underlying reason, just reference seaborn, or leave the value without explanation?

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries, was just curious.

luminance threshold for determining text color. Facilitates text
visibility across varying background colors. From 0 to 1.
0 = all text is dark colored, 1 = all text is light colored.
Copy link
Member

Choose a reason for hiding this comment

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

Can add a version added for 0.24.0 for this

Copy link
Contributor

Choose a reason for hiding this comment

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

is this the common name for this field in mpl?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO text_color=0.2 looks very counterintuitive to me. It almost looks like the exact opposite of what this feature is about (not having a constant text color).
Shouldn't the name at least contain "threshold", e.g. text_color_threshold?

Copy link
Contributor Author

@joelostblom joelostblom May 31, 2018

Choose a reason for hiding this comment

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

I agree that text_color_threshold is suitable, is it ok with multiple underscores in the parameter name? I don't think there is a common name for this in mpl (for coloring text in general, they tend to use just color, but I believe it would be easy to mistake that for the backgruond color here due to the method's name)


.. versionadded:: 0.24.0

Returns
-------
self : Styler

Notes
-----
Tune ``low`` and ``high`` to keep the text legible by
not using the entire range of the color map. These extend
the range of the data by ``low * (x.max() - x.min())``
and ``high * (x.max() - x.min())`` before normalizing.
Set ``text_color_threshold`` or tune ``low`` and ``high`` to keep the
text legible by not using the entire range of the color map. The range
of the data is extended by ``low * (x.max() - x.min())`` and ``high *
(x.max() - x.min())`` before normalizing.

Raises
------
ValueError
If ``text_color_threshold`` is not a value from 0 to 1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Single backtick for parameter names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I did double tick because I saw it in other places, for example for high and low in the notes section. Do you want me to update them to single tick as well? Does the same go for data in the subset section and the expressions in the note section?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I noticed that after posting. Probably fine to be consistent w/ the rest of the docstring here.

"""
subset = _maybe_numeric_slice(self.data, subset)
subset = _non_reducing_slice(subset)
self.apply(self._background_gradient, cmap=cmap, subset=subset,
axis=axis, low=low, high=high)
axis=axis, low=low, high=high,
text_color_threshold=text_color_threshold)
return self

@staticmethod
def _background_gradient(s, cmap='PuBu', low=0, high=0):
def _background_gradient(s, cmap='PuBu', low=0, high=0,
text_color_threshold=0.408):
"""Color background in a range according to the data."""
with _mpl(Styler.background_gradient) as (plt, colors):
rng = s.max() - s.min()
Expand All @@ -909,8 +922,39 @@ def _background_gradient(s, cmap='PuBu', low=0, high=0):
# https://github.com/matplotlib/matplotlib/issues/5427
normed = norm(s.values)
c = [colors.rgb2hex(x) for x in plt.cm.get_cmap(cmap)(normed)]
return ['background-color: {color}'.format(color=color)
for color in c]
if (not isinstance(text_color_threshold, (float, int)) or
not 0 <= text_color_threshold <= 1):
msg = "`text_color_threshold` must be a value from 0 to 1."
raise ValueError(msg)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test to ensure this raises?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am having troubles getting the test correct for this. When I try the function manually, it raises ValueError when called with any one of the parameters in the test, but pytest keeps failing saying that it doesn't raise a ValueError. Would you have time to check my latest commits and advise?


def relative_luminance(color):
"""
Calculate relative luminance of a color.

The calculation adheres to the W3C standards
(https://www.w3.org/WAI/GL/wiki/Relative_luminance)

Parameters
----------
color : matplotlib color
Hex code, rgb-tuple, or HTML color name.

Returns
-------
float
The relative luminance as a value from 0 to 1
"""
rgb = colors.colorConverter.to_rgba_array(color)[:, :3]
rgb = np.where(rgb <= .03928, rgb / 12.92,
((rgb + .055) / 1.055) ** 2.4)
lum = rgb.dot([.2126, .7152, .0722])
return lum.item()

text_colors = ['#f1f1f1' if relative_luminance(x) <
text_color_threshold else '#000000' for x in c]

return ['background-color: {color};color: {tc}'.format(
color=color, tc=tc) for color, tc in zip(c, text_colors)]

def set_properties(self, subset=None, **kwargs):
"""
Expand Down
33 changes: 32 additions & 1 deletion pandas/tests/io/formats/test_style.py
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,38 @@ def test_background_gradient(self):

result = df.style.background_gradient(
subset=pd.IndexSlice[1, 'A'])._compute().ctx
assert result[(1, 0)] == ['background-color: #fff7fb']

assert result[(1, 0)] == ['background-color: #fff7fb',
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a more comprehensive / dedicated test for this? Something that encompasses the full range of expected values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a suggestion

'color: #000000']

@td.skip_if_no_mpl
Copy link
Member

Choose a reason for hiding this comment

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

Do these tests need to be in the MatplotlibDep test class? I didn't think this required mpl but could be wrong.

If that is the case then move the @td.skip_if_no_mpl decorator to the class instead of on each function individually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, they both call background_gradient() which depends on mpl.

@pytest.mark.parametrize("c_map", [None, 'YlOrRd'])
def test_text_color_threshold(self, c_map):
df = pd.DataFrame([[1, 2], [2, 4]], columns=['A', 'B'])
result = df.style.background_gradient(cmap=c_map)._compute().ctx
test_colors = {None: {(0, 0): ('#440154', '#f1f1f1'),
Copy link
Member

Choose a reason for hiding this comment

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

Change the variable here to expected (standard in pandas tests)

Copy link
Member

Choose a reason for hiding this comment

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

Instead of using a nested dict you should also send this in via parametrization, so you would have "c_map,expected" and then send in a tuple of the c_map and it's expected result

(1, 0): ('#fde725', '#000000')},
'YlOrRd': {(0, 0): ('#ffffcc', '#000000'),
(1, 0): ('#800026', '#f1f1f1')}}
# Light text on dark background
assert result[0, 0][0].split(' ')[1] == test_colors[c_map][0, 0][0], (
Copy link
Member

Choose a reason for hiding this comment

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

Why are you splitting this? Just make your expected variable account for the exact string required. Unless I'm missing something you should just have one assertion here for result == expected

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was doing this to be able to raise different assertion messages for the background and foreground color. But since pytest shows the expected and current value anyways, this should be easy to trace done without separate assertion messages.

'Unexpected background color returned from '
'`style.background_gradient()`')
assert result[0, 0][1].split(' ')[1] == test_colors[c_map][0, 0][1]
# Dark text on light background
assert result[1, 0][0].split(' ')[1] == test_colors[c_map][1, 0][0], (
'Unexpected background color returned from '
'`style.background_gradient()`')
assert result[1, 0][1].split(' ')[1] == test_colors[c_map][1, 0][1]

@td.skip_if_no_mpl
@pytest.mark.parametrize("text_color_threshold", [1.1, '1', -1, [2, 2]])
def test_text_color_threshold_raises(self, text_color_threshold):
df = pd.DataFrame([[1, 2], [2, 4]], columns=['A', 'B'])
msg = "`text_color_threshold` must be a value from 0 to 1."
with tm.assert_raises_regex(ValueError, msg):
df.style.background_gradient(
text_color_threshold=text_color_threshold)._compute()


def test_block_names():
Expand Down