-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add getters and _repr_html_ for over/under/bad values of Colormap objects. #17900
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
Add getters and _repr_html_ for over/under/bad values of Colormap objects. #17900
Conversation
1c633ab
to
cc8318e
Compare
This is what I wondered about in the first PR. over/under are usually in the extended portion of the colorbar which we represent via a triangle on either side: https://matplotlib.org/2.0.2/_images/colorbar_only.png I also wonder if the colorbar needs to be so wide, but that is just an aesthetic choice |
@jklymak I think that the use of color blocks for over, under, and bad is a good idea. It is explicit and complete. Using the actual colorbar with extensions would show under and over, but not bad. |
ThTs fine. I'm certainly not going to block on this at all. I just think it's a little inconsistent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great addition, and will be very useful to check interactively what is currently set for the colormap values!
For the ordering, I am wondering if there is a way to put the "under" box left-aligned and the "over" box right-aligned to put them next to the values they are "near"? Right now, the yellow "over" is a bit far away from the far right edge. (That might mean putting the title on a row above the squares to squeeze it in) I missed the sprint unfortunately, so missed if you already tried this or not.
@greglucas I discussed with @tacaswell and we decided it would be less obtrusive in a notebook if we kept the color map name and under/over/bad boxes on the same line (above the image). Are you suggesting one of layouts B or C? (represented in ASCII style) Layout A (current):
Layout B: left/right aligned with text labels
Layout C: left/right aligned with color swatches
|
Nice, I think it is OK the way it is and I haven't messed with anything else to see how they look. My thought was something along the lines of your third option. Maybe like this?
I agree though, this would add more height to the image, so maybe not as ideal as having them all on the same line. |
Or you could have over/under beside the colorbar (perhaps as triangles) and the bad swatch beside the title. I'd put the title on the left and the bad swatch on the right.
|
Something like this adds a triangle (if you just want to use html/css: https://stackoverflow.com/questions/44109039/how-to-put-a-css-triangle-before-a-span |
@jklymak @greglucas I like the idea, but a triangle implemented with CSS @greglucas's suggestion of
is a viable design, if there is consensus on this being the best option. I am happy to make another revision if we get a stronger consensus on the design. |
I feel like we're treading a tad close to "design by committee" and bike-shedding to death here. I'm 👍 to @greglucas's suggestion. |
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
8a36e2a
to
c43cceb
Compare
@dopplershift @jklymak @greglucas @timhoffm I spent some time trying to make the design mentioned above and got as close as I could. The alignment of the labels below the image is very tricky. Is the image below acceptable (or is someone else with CSS expertise able to help)? Revised appearance |
I would also appreciate help understanding why CircleCI |
We fail on sphinx warnings as well
|
Personally, I would go with either your first version (with the boxes on the same line as the name), or with the version below. I think the thing you were missing was to flex the div and justify the contents of the container. Click for code
def color_block(color):
hex_color = to_hex(color, keep_alpha=True)
return ('<div title="' + hex_color + '" ' +
'style="display: inline-block; ' +
'width: 1em; height: 1em; ' +
'margin: 0; ' +
'vertical-align: middle; ' +
'border: 1px solid #555; ' +
'background-color: ' + hex_color + ';">' +
'</div>')
return ('<div style="vertical-align: middle;">' +
'<strong>' + self.name + '</strong> ' +
'</div>' +
'<div id="cmap"><img ' +
'alt="' + self.name + ' color map" ' +
'title="' + self.name + '"' +
'style="border: 1px solid #555;" ' +
'src="data:image/png;base64,' + png_base64 + '"></div>' +
'<div style="vertical-align: middle; width:402px; ' +
'display:flex; justify-content:space-between;">' +
'<div style="float:left;">' +
color_block(self.get_under()) + ' under' +
'</div>' +
'<div style="margin:0 auto; display:inline-block;">' +
'bad ' + color_block(self.get_bad()) +
'</div>' +
'<div style="float: right;">' +
'over ' + color_block(self.get_over()) +
'</div>') |
Thanks @greglucas and @jklymak, that helped a lot. I also got the content to be responsive in narrow windows ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to add a test for the _repr_html_
function? Assert that it is returning the html expected in a specific case or something similar.
Looks good to me! Only the few minor questions.
@greglucas I improved the HTML tests. Checking against a specific output is probably not ideal because the PNG is directly embedded as base64 and the HTML output for viridis is ~2,104 characters. It wouldn't fit well into a test file. <div style="vertical-align: middle;"><strong>viridis</strong> </div><div class="cmap"><img alt="viridis color map" title="viridis" style="border: 1px solid #555;" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAgAAAABACAYAAABsv8+/AAAAF3RFWHRUaXRsZQB2aXJpZGlzIGNvbG9yIG1hcCbEUIsAAAAddEVYdERlc2NyaXB0aW9uAHZpcmlkaXMgY29sb3IgbWFwD7q3ogAAAEt0RVh0QXV0aG9yAE1hdHBsb3RsaWIgdjMuMy4wcmMxLnBvc3Q2NDMuZGV2MCtnNWIwOTVmOTNhLCBodHRwczovL21hdHBsb3RsaWIub3JnpWoE0wAAAE10RVh0U29mdHdhcmUATWF0cGxvdGxpYiB2My4zLjByYzEucG9zdDY0My5kZXYwK2c1YjA5NWY5M2EsIGh0dHBzOi8vbWF0cGxvdGxpYi5vcmdUElJHAAACIklEQVR4nO3WQZKbMBRF0S/YWpaQ/S8l9CAyFAIZx5XZO2fikvQRVA+6bvvVfm9VVdVa/f1d+s95XX3dXnNL3x/Wx/n98+N7LvfO7r/ce/+7PZzPnt/28/ryvE737/Ot7ueG+8b563q8f3ZvDc8N9ywP55fn37/nef3p/ffrb9/zv+9/nKv3c5/vb989X0/z21ffMZ4fc5Pz/Ttmz7/fb5O5dlkPz9U4N/xbm64f9vu9y2V+6/s1rMfz+3tmc/tvPZwPc0v78+HceX795/Pze9a6f++4v76eq4f1673D+67r++847j1//zF/vu86P7xvtj98x+x71xr/Lue/53rZr75f5/W+34b9vu77+7pPHPNL/+1zbTndAwAEEQAAEEgAAEAgAQAAgQQAAAQSAAAQSAAAQCABAACBBAAABBIAABBIAABAIAEAAIEEAAAEEgAAEEgAAEAgAQAAgQQAAAQSAAAQSAAAQCABAACBBAAABBIAABBIAABAIAEAAIEEAAAEEgAAEEgAAEAgAQAAgQQAAAQSAAAQSAAAQCABAACBBAAABBIAABBIAABAIAEAAIEEAAAEEgAAEEgAAEAgAQAAgQQAAAQSAAAQSAAAQCABAACBBAAABBIAABBIAABAIAEAAIEEAAAEEgAAEEgAAEAgAQAAgQQAAAQSAAAQSAAAQCABAACBBAAABBIAABBIAABAIAEAAIEEAAAEEgAAEEgAAECgH25wiYfFwpm0AAAAAElFTkSuQmCC"></div><div style="vertical-align: middle; max-width: 514px; display: flex; justify-content: space-between;"><div style="float: left;"><div title="#440154ff" style="display: inline-block; width: 1em; height: 1em; margin: 0; vertical-align: middle; border: 1px solid #555; background-color: #440154ff;"></div> under</div><div style="margin: 0 auto; display: inline-block;">bad <div title="#00000000" style="display: inline-block; width: 1em; height: 1em; margin: 0; vertical-align: middle; border: 1px solid #555; background-color: #00000000;"></div></div><div style="float: right;">over <div title="#fde725ff" style="display: inline-block; width: 1em; height: 1em; margin: 0; vertical-align: middle; border: 1px solid #555; background-color: #fde725ff;"></div></div></div> |
@QuLogic is there a reason you did not merge? Should be squashed to one or a few commits. |
I restarted tests, I believe. Probably the macOS/TkAgg issue. |
@bdice Thanks for your contribution and the patience to work though the various review iterations! We hope to see you back some time. |
PR Summary
This PR extends from #17888 to add over/under/bad values in the
_repr_html_
ofmatplotlib.colors.Colormap
objects. This also adds public getters for those values, which previously had only setters.Demo
PR Checklist