Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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.
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. | |||
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.
Suspect at least this docstring needs more info....
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.
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.
lib/matplotlib/contour.py Outdated
| except AttributeError: | ||
| pass | ||
| under = np.nonzero(lev < self.zmin)[0] |
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.
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...
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.
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. |
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.
"... in data space"?
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.
No, it is screen space. That's why the geometric mean is needed for the log scale.
lib/matplotlib/tests/test_axes.py Outdated
| @@ -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=['/', '\\', '//', '-'], | |||
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.
Could we mpl20 and remove-text=True this example while we are changing the image?
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. |
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.
LGTM....
The last round passed the image tests, so this is now squashed. |
Great - if you rebase, it'll pass CI as well (I assume); see#11500 |
Nominally 👍 , but want to look at images first (currently on 4g). Do we expect a big back-lash on this? |
I don't expect major complaints. |
@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 overruncan 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 miminumrequired to include the data interval in the default case, andwill be reduced when "extend" is used so that some of the datarange will fall in the extended sections. This is expected tobe rare, however; normally the "extend" kwarg would be used onlywhen levels are explicitly set, not auto-selected with a Locator.The contour_hatching test results were modified. The differencewas reduced by putting the hatches in the same order as they were in thereference figure, but the images still don't match becausethe gray shades have changed, as expected.(Note that in the original and new reference figuresthe pdf and svg backends handle the alpha differently thanthe agg backend. This is unrelated to the PR.)For logscale contour colors, interpolate in screen spaceby taking the geometric mean of adjacent levels.The test_contour.py::test_contourf_log_extension code andimage have been modified for this change in colors, and tomake the behavior more clear in the case where levels arenot specified.Add a note to next_api_changes.Treat contour levels (almost) the same as contourf levelsUpdate additional test images affected by the contour color changes.mplot3d: don't assume every contour level has a line.
These changes are intended, with better selection of contour levels,cf.matplotlib/matplotlib#11412.
These changes are intended, with better selection of contour levels,cf.matplotlib/matplotlib#11412.
These changes are intended, with better selection of contour levels,cf.matplotlib/matplotlib#11412.
These changes are intended, with better selection of contour levels,cf.matplotlib/matplotlib#11412.
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
levelsattribute is replaced with a list holdingonly 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
LogNormor aLogLocator, it will nowselect 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