Skip to content

fix annotations font color #2892

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

fix annotations font color #2892

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
48b533d
fix annotations font color
ben-dem Nov 11, 2020
b5c7cff
black formatting
ben-dem Nov 11, 2020
55ab39d
remove my test file
ben-dem Nov 11, 2020
413eed6
black reformatting
ben-dem Nov 11, 2020
ae1f25f
update docstring
ben-dem Nov 11, 2020
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
40 changes: 26 additions & 14 deletions packages/python/plotly/plotly/figure_factory/_annotated_heatmap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,9 @@ def create_annotated_heatmap(
# validate colorscale
colorscale_validator = ColorscaleValidator()
colorscale = colorscale_validator.validate_coerce(colorscale)

annotations = _AnnotatedHeatmap(
z, x, y, annotation_text, colorscale, font_colors, reversescale, **kwargs
).make_annotations()
).make_annotations(**kwargs)

if x or y:
trace = dict(
Expand Down Expand Up @@ -136,7 +135,7 @@ def create_annotated_heatmap(
layout = dict(
annotations=annotations,
xaxis=dict(
ticks="", side="top", gridcolor="rgb(0, 0, 0)", showticklabels=False
ticks="", side="top", gridcolor="rgb(0, 0, 0)", showticklabels=False,
),
yaxis=dict(ticks="", ticksuffix=" ", showticklabels=False),
)
Expand Down Expand Up @@ -211,7 +210,6 @@ def get_text_color(self):
"Blues",
"YIGnBu",
"YIOrRd",
"RdBu",
"Picnic",
"Jet",
"Hot",
Expand Down Expand Up @@ -264,34 +262,48 @@ def get_text_color(self):
max_text_color = black
return min_text_color, max_text_color

def get_z_mid(self):
def get_z_min_max(self):
"""
Get the mid value of z matrix
Get the min and max value of z matrix

:rtype (float) z_avg: average val from z matrix
:rtype (tuple): min and max val from z matrix
"""
if np and isinstance(self.z, np.ndarray):
z_min = np.amin(self.z)
z_max = np.amax(self.z)
z_min = np.nanmin(self.z)
z_max = np.nanmax(self.z)
else:
z_min = min([v for row in self.z for v in row])
z_max = max([v for row in self.z for v in row])
z_mid = (z_max + z_min) / 2
return z_mid
return z_min, z_max

def make_annotations(self):
def make_annotations(self, zmin=None, zmid=None, zmax=None):
"""
Get annotations for each cell of the heatmap with graph_objs.Annotation

:rtype (list[dict]) annotations: list of annotations for each cell of
the heatmap
"""
min_text_color, max_text_color = _AnnotatedHeatmap.get_text_color(self)
z_mid = _AnnotatedHeatmap.get_z_mid(self)
if zmin is None or zmax is None:
zmin, zmax = _AnnotatedHeatmap.get_z_min_max(self)
Copy link
Contributor

Choose a reason for hiding this comment

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

here it seems like we overwrite both if either is missing? probably best not to do that I think.

Copy link
Author

Choose a reason for hiding this comment

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

From what I understand from the doc https://plotly.github.io/plotly.py-docs/generated/plotly.graph_objects.Heatmap.html "zmax – Sets the upper bound of the color domain. Value should have the same units as in z and if set, zmin must be set as well." we can not pass only one of these 2 thats why i overwrite both

Copy link
Contributor

Choose a reason for hiding this comment

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

This is true of the underlying heatmap trace, yes, but this figure-factory already does its own bounds-computation, so we may as well reuse it. I've already addressed this comment in #2921.

if zmid is None:
zmid = (zmax + zmin) / 2
if min_text_color == max_text_color:
# diverging colorscale
mid_text_color = "#000000"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this is a great default... this is assuming the diverging scale is light in the middle?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we shouldn't accept a third color in font_colors for this and only set this one here if it's not present?

Copy link
Author

Choose a reason for hiding this comment

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

Yes can be an option but is there any diverging colorscale where the it's not the case?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, users can pass in their own colorscale, they're not limited to just the built-in ones.

get_font_color = (
lambda val: mid_text_color
if (zmin + zmid) / 2 < val < (zmid + zmax) / 2
else min_text_color
)
else:
get_font_color = (
lambda val: min_text_color if val < zmid else max_text_color
)
annotations = []
for n, row in enumerate(self.z):
for m, val in enumerate(row):
font_color = min_text_color if val < z_mid else max_text_color
font_color = get_font_color(val)
annotations.append(
graph_objs.layout.Annotation(
text=str(self.annotation_text[n][m]),
Expand Down