Skip to content

Make contour and contourf color assignments consistent. #11412

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 1 commit into from
Jul 8, 2018

Conversation

efiring
Copy link
Member

@efiring efiring commented Jun 10, 2018

PR Summary

This is derived from, and replaces, #8834.

Selection of contour levels is now the same for contour and
contourf; previously, for contour, levels outside the data range were
deleted. (Exception: if no contour levels are found within the
data range, the levels attribute is replaced with a list holding
only the minimum of the data range.)

When contour is called with levels specified as a target number rather
than a list, and the 'extend' kwarg is used, the levels are now chosen
such that some data typically will fall in the extended range.

When contour is called with a LogNorm or a LogLocator, it will now
select colors using the geometric mean rather than the arithmetic mean
of the contour levels.

Closes #10572.

An unfortunate side effect is that many test images have to be changed because they happen to use colored line contours generated via the minimal call to contour for which the behavior is changed by this PR.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 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

@jklymak jklymak self-assigned this Jun 11, 2018
@jklymak jklymak self-requested a review June 11, 2018 19:25
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Overall seems fine.

  • does the contour docs need to be updated to specify the level algorithm?
  • Should the breaking tests be modified to not all use auto-levels?

@@ -1180,15 +1180,34 @@ def _autolev(self, N):
one contour line, but two filled regions, and therefore
three levels to provide boundaries for both regions.
Copy link
Member

Choose a reason for hiding this comment

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

Suspect at least this docstring needs more info....

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what info you are looking for, but I am adding some now. This is a private method, so the docstring requirements are not as stringent as they would be for a public method.

except AttributeError:
pass

under = np.nonzero(lev < self.zmin)[0]
Copy link
Member

Choose a reason for hiding this comment

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

This code block could use a comment - I didn't find it easy to parse and would only have known because you explained it to me on the weekly call...

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.

self.layers[0] = -1e150
if self.extend in ('both', 'max'):
self.layers[-1] = 1e150
# Layer values are mid-way between levels in screen space.
Copy link
Member

Choose a reason for hiding this comment

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

"... in data space"?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it is screen space. That's why the geometric mean is needed for the log scale.

@@ -1647,7 +1647,7 @@ def test_contour_hatching():

fig = plt.figure()
ax = fig.add_subplot(111)
cs = ax.contourf(x, y, z, hatches=['-', '/', '\\', '//'],
cs = ax.contourf(x, y, z, hatches=['/', '\\', '//', '-'],
Copy link
Member

Choose a reason for hiding this comment

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

Could we mpl20 and remove-text=True this example while we are changing the image?

@efiring
Copy link
Member Author

efiring commented Jul 3, 2018

This will need to be squashed before merging so that the earlier set of images doesn't clutter the history. I will do that after seeing that the new images work; I don't trust my local setup to generate images that match those of the test environment.

Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

LGTM....

@efiring
Copy link
Member Author

efiring commented Jul 3, 2018

The last round passed the image tests, so this is now squashed.

@jklymak
Copy link
Member

jklymak commented Jul 3, 2018

Great - if you rebase, it'll pass CI as well (I assume); see #11500

@tacaswell tacaswell added this to the v3.0 milestone Jul 4, 2018
@tacaswell
Copy link
Member

Nominally 👍 , but want to look at images first (currently on 4g).

Do we expect a big back-lash on this?

@efiring
Copy link
Member Author

efiring commented Jul 4, 2018

I don't expect major complaints.

@tacaswell
Copy link
Member

@efiring can you rebase this?

Contour level auto-selection: clip unused Locator output:

Locator.tick_values() returns levels beyond the data limits.
In the case of LogLocator with a small data range, the overrun
can be large because it is expanding to the decade points.
In addition, no account was being taken of the "extend" kwarg.
With this changeset, the outermost levels will be the miminum
required to include the data interval in the default case, and
will be reduced when "extend" is used so that some of the data
range will fall in the extended sections.  This is expected to
be rare, however; normally the "extend" kwarg would be used only
when levels are explicitly set, not auto-selected with a Locator.

The contour_hatching test results were modified.  The difference
was reduced by putting the hatches in the same order as they were in the
reference figure, but the images still don't match because
the gray shades have changed, as expected.

(Note that in the original and new reference figures
the pdf and svg backends handle the alpha differently than
the agg backend.  This is unrelated to the PR.)

For logscale contour colors, interpolate in screen space
by taking the geometric mean of adjacent levels.

The test_contour.py::test_contourf_log_extension code and
image have been modified for this change in colors, and to
make the behavior more clear in the case where levels are
not specified.

Add a note to next_api_changes.

Treat contour levels (almost) the same as contourf levels

Update additional test images affected by the contour color changes.

mplot3d: don't assume every contour level has a line.
@jklymak jklymak merged commit 40cfdae into matplotlib:master Jul 8, 2018
@efiring efiring deleted the contour_colors branch July 9, 2018 07:36
QuLogic added a commit to QuLogic/cartopy that referenced this pull request Sep 26, 2018
These changes are intended, with better selection of contour levels,
cf. matplotlib/matplotlib#11412.
QuLogic added a commit to QuLogic/cartopy that referenced this pull request Sep 26, 2018
These changes are intended, with better selection of contour levels,
cf. matplotlib/matplotlib#11412.
QuLogic added a commit to QuLogic/cartopy that referenced this pull request Nov 8, 2018
These changes are intended, with better selection of contour levels,
cf. matplotlib/matplotlib#11412.
QuLogic added a commit to QuLogic/cartopy that referenced this pull request Nov 8, 2018
These changes are intended, with better selection of contour levels,
cf. matplotlib/matplotlib#11412.
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.

contour and contourf treat levels differently
3 participants