Skip to content

Add the "contour.linewidths" configuration option #17236

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 7 commits into from
May 6, 2020

Conversation

stefraynaud
Copy link
Contributor

@stefraynaud stefraynaud commented Apr 24, 2020

PR Summary

Contours plots are often more beautiful when line widths are smaller
than line widths that are generally used for curves.
This is especially true when contour is used with contourf, or when
the number of levels if high.

With this PR, one can change default value for contour linewidths independently from normal line widths. The new options is "contour.linewidths", which defaults to None. When value is None, it falls back to "lines.linewidth" like before.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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/api_changes.rst if API changed in a backward-incompatible way

It defaults to None whcih makes it to fallback
to "lines.linewidth" option.
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Looks good! Only a few minor comments.

@stefraynaud
Copy link
Contributor Author

Linting test is now is failing because of the "default: :rc..." which makes the line too long.

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

This should get a what's new entry: Add a file to /doc/users/next_whats_new (see also README.rst in there).

Other than that, this is good to go.

@QuLogic
Copy link
Member

QuLogic commented May 4, 2020

Is it plural linewidths, or singular linewidth? The what's new entry uses both.

@stefraynaud
Copy link
Contributor Author

@QuLogic It's the plural like the argument to contour(). So I made a mistake in the what new entry.

@QuLogic
Copy link
Member

QuLogic commented May 6, 2020

OK, it only accepts a single float, though? So it should not be plural.

@timhoffm
Copy link
Member

timhoffm commented May 6, 2020

I agree with @QuLogic

While the argument is linewidths (think: the linewidths of the contours, which can be a list), the rcParam should indeed be contour.linewidth (think: the default linewidth of the contour).

@stefraynaud
Copy link
Contributor Author

I agree with you both. My concern was about not adding confusion in the user's mind, despite it's less logical to name it linewidths.
So I renamed it.

@QuLogic QuLogic merged commit cd755c4 into matplotlib:master May 6, 2020
@QuLogic QuLogic added this to the v3.3.0 milestone May 6, 2020
@stefraynaud stefraynaud deleted the default_contour_linewidth branch May 7, 2020 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants