Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

bdice
Copy link
Contributor

@bdicebdice commentedJul 12, 2020
edited
Loading

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

image

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

@bdicebdice marked this pull request as ready for reviewJuly 12, 2020 19:19
@bdicebdiceforce-pushed thecolormap-repr-under-over-bad branch from1c633ab tocc8318eCompareJuly 12, 2020 19:21
@tacaswelltacaswell added this to thev3.4.0 milestoneJul 12, 2020
@jklymak
Copy link
Member

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

@efiring
Copy link
Member

@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.

tacaswell reacted with thumbs up emoji

@jklymak
Copy link
Member

ThTs fine. I'm certainly not going to block on this at all. I just think it's a little inconsistent.

Copy link
Contributor

@greglucasgreglucas left a 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.

bdice reacted with thumbs up emoji
@bdice
Copy link
ContributorAuthor

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):

viridis  under: [█]  over: [█]  bad: [█][████████████████████████████████████████]

Layout B: left/right aligned with text labels

viridis                           bad: [█][████████████████████████████████████████]under: [█]                       over: [█]

Layout C: left/right aligned with color swatches

viridis                            bad [█][████████████████████████████████████████][█] under                         over [█]

@bdicebdice requested a review fromtimhoffmJuly 13, 2020 00:10
@greglucas
Copy link
Contributor

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?

                viridis[████████████████████████████████████████][█] under       bad [█]           over [█]

I agree though, this would add more height to the image, so maybe not as ideal as having them all on the same line.

@jklymak
Copy link
Member

jklymak commentedJul 13, 2020
edited
Loading

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.

viridis                            bad [█]< [████████████████████████████████████████] >
greglucas reacted with thumbs up emoji

@jklymak
Copy link
Member

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

@bdice
Copy link
ContributorAuthor

@jklymak@greglucas I like the idea, but a triangle implemented with CSSborder properties will not work with the thin grey line that denotes the edge of the color map image (which also usesborder). I don't think you could put the thin grey line around the triangle.@tacaswell suggested adding that border because some color maps may fade to white (or transparent) on one end, and showing the boundary of the image helps with that.

@greglucas's suggestion of

                viridis[████████████████████████████████████████][█] under       bad [█]           over [█]

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.

@dopplershift
Copy link
Contributor

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.

@bdicebdiceforce-pushed thecolormap-repr-under-over-bad branch from8a36e2a toc43ccebCompareAugust 9, 2020 03:47
@bdice
Copy link
ContributorAuthor

@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

image

@bdice
Copy link
ContributorAuthor

I would also appreciate help understanding why CircleCIdocs-python3* is failing. I don't see any error messages in the CI log...?

@jklymak
Copy link
Member

We fail on sphinx warnings as well

/home/circleci/project/doc/users/next_whats_new/colormap_get_under_over_bad.rst:4: WARNING: py:obj reference target not found: colors.Colormap.get_under/home/circleci/project/doc/users/next_whats_new/colormap_get_under_over_bad.rst:4: WARNING: py:obj reference target not found: colors.Colormap.get_over/home/circleci/project/doc/users/next_whats_new/colormap_get_under_over_bad.rst:4: WARNING: py:obj reference target not found: colors.Colormap.get_bad
bdice reacted with thumbs up emoji

@greglucas
Copy link
Contributor

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.'display:flex; justify-content:space-between;

With that I get this image:
Screen Shot 2020-08-09 at 3 17 42 PM

Click for code

defcolor_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>'+'<strong>'+self.name+'</strong> '+'</div>'+'<div><img '+'alt="'+self.name+' color map" '+'title="'+self.name+'"'+'style="border: 1px solid #555;" '+'src="data:image/png;base64,'+png_base64+'"></div>'+'<div>'+'<div>'+color_block(self.get_under())+' under'+'</div>'+'<div>'+'bad '+color_block(self.get_bad())+'</div>'+'<div>'+'over '+color_block(self.get_over())+'</div>')

bdice reacted with thumbs up emoji

@bdice
Copy link
ContributorAuthor

bdice commentedAug 10, 2020
edited
Loading

Thanks@greglucas and@jklymak, that helped a lot. I also got the content to be responsive in narrow windows (max-width instead ofwidth). This PR should be ready for final review! 😄

image

Copy link
Contributor

@greglucasgreglucas left a 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.

@bdice
Copy link
ContributorAuthor

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.

@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.

<divstyle="vertical-align: middle;"><strong>viridis</strong></div><divclass="cmap"><imgalt="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><divstyle="vertical-align: middle; max-width: 514px; display: flex; justify-content: space-between;"><divstyle="float: left;"><divtitle="#440154ff"style="display: inline-block; width: 1em; height: 1em; margin: 0; vertical-align: middle; border: 1px solid #555; background-color: #440154ff;"></div> under</div><divstyle="margin: 0 auto; display: inline-block;">bad<divtitle="#00000000"style="display: inline-block; width: 1em; height: 1em; margin: 0; vertical-align: middle; border: 1px solid #555; background-color: #00000000;"></div></div><divstyle="float: right;">over<divtitle="#fde725ff"style="display: inline-block; width: 1em; height: 1em; margin: 0; vertical-align: middle; border: 1px solid #555; background-color: #fde725ff;"></div></div></div>
greglucas reacted with thumbs up emoji

@bdicebdice requested a review fromtimhoffmAugust 16, 2020 03:02
@timhoffm
Copy link
Member

timhoffm commentedAug 18, 2020
edited
Loading

@QuLogic is there a reason you did not merge?

Should be squashed to one or a few commits.

@QuLogic
Copy link
Member

I restarted tests, I believe. Probably the macOS/TkAgg issue.

@timhoffmtimhoffm merged commit13e3573 intomatplotlib:masterAug 19, 2020
@timhoffm
Copy link
Member

@bdice Thanks for your contribution and the patience to work though the various review iterations! We hope to see you back some time.

bdice reacted with thumbs up emojibdice reacted with rocket emoji

@bdicebdice deleted the colormap-repr-under-over-bad branchAugust 19, 2020 16:13
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@greglucasgreglucasgreglucas left review comments

@QuLogicQuLogicQuLogic approved these changes

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

8 participants
@bdice@jklymak@efiring@greglucas@dopplershift@timhoffm@QuLogic@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp