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

Fix edge color, links, wording; closes matplotlib/matplotlib#23895#23930

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
tacaswell merged 3 commits intomatplotlib:mainfrombaharev:main
Sep 20, 2022
Merged

Conversation

@baharev
Copy link
Contributor

PR Summary

See#23895:

  • the 3D surface was unrecognizable due to the invisible edges,
  • the links in the documentation were broken,
  • the wording in the title did not conform to the guidelines.

Closes#23895.

PR Checklist

Tests and Styling

  • [N/A] Has pytest style unit tests (andpytest passes).
  • [N/A] IsFlake 8 compliant (installflake8-docstrings and runflake8 --docstring-convention=all).

Documentation

  • [N/A] New features are documented, with examples if plot related.
  • [N/A] New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).
  • [N/A] Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).

Copy link

@github-actionsgithub-actionsbot left a comment

Choose a reason for hiding this comment

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

Thank you for opening your first PR into Matplotlib!

If you have not heard from us in a while, please feel free to ping@matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.

You can also join uson gitter for real-time discussion.

For details on testing, writing docs, and our review process, please seethe developer guide

We strive to be a welcoming and open project. Please follow ourCode of Conduct.

@tacaswell
Copy link
Member

Thank you for finding and fixing this very old fallout from the mpl 1->2 style change!

@baharev
Copy link
ContributorAuthor

@tacaswell Thanks for the feedback.

I think it would look a bit better with a thinner lw on the surface?

Agreed, those plots are ugly. However, this is also very strange: It is not how it looks like on my machine locally.

How should I fix this?

@tacaswell
Copy link
Member

Do you have any custom rcparams set?

The simplest solution is to pass lw to the calls, but we should understand why you get something different locally.

@timhoffm
Copy link
Member

I did not check whether it's related, buthttps://matplotlib.org/devdocs/gallery/mplot3d/wire3d.html#sphx-glr-gallery-mplot3d-wire3d-py has a similar linewidth.

@oscargus
Copy link
Member

It would indeed look better with thinner lines, but then this seems to be the default linewidth considering the width of the other plotted lines in that figure (the ones on the panes)?

@timhoffm
Copy link
Member

ThePoly3DCollection usespatch.linewidth: 1.0 by default.

For comparisongrid.linewidth: 0.8. So yes, the lines are thicker. I suppose the lines appear even more prominent because of the black color.

I suggest to make the lines thinner (0.8 or even 0.5) and maybe go from black to a dark grey. We already a lot of customization on theplot_surface call. I'm therefore not worried adding more to make the plot look nicer.

@baharev
Copy link
ContributorAuthor

I am someone with serious color vision deficiency, so I am kindly asking you to vote. Which one of these looks the best?

Each withlw=0.5

edgecolor='black'

black

edgecolor='cm.coolwarm(0)'

cmap_0

edgecolor='0.35'

035

I find the one in the middle the nicest, but I have serious trouble with colors. So, please vote.

@timhoffm
Copy link
Member

I find the one in the middle the nicest.

I agree. There's some consistency in having darker blue lines on a light-blue surface. Could you also trytab:blue as a color? I think the faces aretab:blue but light because of the transparency.

@baharev
Copy link
ContributorAuthor

Thank you for your prompt feedback. Here is the one withtab:blue:

tabblue

I find the one withedgecolor='cm.coolwarm(0)' a bit more vivid (nicer).

@timhoffm
Copy link
Member

Thanks!

I find the one withedgecolor='cm.coolwarm(0)' a bit more vivid (nicer).

Agreed. Let's use that one.

@baharev
Copy link
ContributorAuthor

Done. All green. I do not know how to link the built updated pages here, sorry. They look really nice locally on my machine.

frommpl_toolkits.mplot3dimportaxes3d
importmatplotlib.pyplotasplt
frommatplotlibimportcm
frommatplotlib.cmimportcoolwarmascmap
Copy link
Member

Choose a reason for hiding this comment

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

It's recommended nowadays to use the colormap registry instead of importing frommatplotlib.cm:

cmap = plt.colormaps['coolwarm']

You could go even one step further and use the stringcontour(..., cmap='coolwarm'). Usually, that's the simplest way if you don't need the colormap object itself. Here however, you have thecmap(0) call. I'm tempted to resolve this to an explicit'#3b4cc0' and get rid of the colormap object completely, which IMHO makes the example simpler.

But I'll let you choose whether to go all-string or retrievecmap fromplt.colormaps.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, thatcmap(0) is bugging me too. I was considering'#3b4cc0' too, but it is too cryptic. Is there some named blue in the neighborhood?

https://matplotlib.org/stable/gallery/color/named_colors.html#css-colors

Since I am green blind, I would like to kindly ask you to please help me out here: If you agree, please suggest a named blue instead of'#3b4cc0'. Thanks!

Copy link
Member

Choose a reason for hiding this comment

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

image

You can try 'mediumblue' which is slightly darker, or 'royalblue' which is a bit lighter.

frommpl_toolkits.mplot3dimportaxes3d
importmatplotlib.pyplotasplt
frommatplotlibimportcm
frommatplotlib.cmimportcoolwarmascmap
Copy link
Member

Choose a reason for hiding this comment

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

As above.

@timhoffm
Copy link
Member

I do not know how to link the built

FYI: There's a "View the built docs" entry in the CI checks section below:

image

@baharev
Copy link
ContributorAuthor

@tacaswell
Copy link
Member

Thank you@baharev and congratulations on your first merged Matplotlib PR 🎉

I hope that we will hear from you again!

You maybe interested in
https://www.youtube.com/watch?v=Gapv8wR5DYU&list=PLYx7XA2nY5Ge3LsWy500pi5bdHEiAdQB5&index=4
Development of Accessible, Aesthetically-Pleasing Color Sequences - Matthew Petroff (@mpetroff) from Scipy 2022.

One of my regrets from the 2.0 default style changes is that while we put a whole lot of effort into the color map and making sure it was OK for color vision deficiencies, we failed to do the same to the default color cycle.

@baharev
Copy link
ContributorAuthor

And I would like to thank you all for your help, patience and constructive feedback. 🙂

I will watch that video this evening, thanks for the info.

oscargus added a commit that referenced this pull requestSep 21, 2022
…930-on-v3.6.xBackport PR#23930 on branch v3.6.x (Fix edge color, links, wording;closes#23895)
@ksundenksunden mentioned this pull requestFeb 20, 2023
6 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@timhoffmtimhoffmtimhoffm approved these changes

@github-actionsgithub-actions[bot]github-actions[bot] left review comments

@tacaswelltacaswelltacaswell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.6.1

Development

Successfully merging this pull request may close these issues.

[Bug]: 3D surface is not plotted for the contour3d_3 example in the gallery

4 participants

@baharev@tacaswell@timhoffm@oscargus
[8]ページ先頭
©2009-2025 Movatter.jp