Skip to content

Fix specifying number of levels with log contour#27576

Merged
dstansby merged 8 commits intomatplotlib:mainfrom
dstansby:log-contour-levels
Apr 30, 2025
Merged

Fix specifying number of levels with log contour#27576
dstansby merged 8 commits intomatplotlib:mainfrom
dstansby:log-contour-levels

Conversation

@dstansby
Copy link
Copy Markdown
Member

@dstansby dstansby commented Dec 28, 2023 •
edited
Loading

PR summary

When plotting contours with a log norm, passing an integer value to the levels argument to cap the maximum number of contour levels now works as intended. Partially addresses #19856 and replaces #25149.

I've maually checked that the added test fails before this fix. Testing with the following script:

import matplotlib.pyplot as plt
from matplotlib.colors import LogNorm
import numpy as np

x, y = np.mgrid[1:10:0.1, 1:10:0.1]
data = np.abs(np.sin(x)*np.exp(y))

for n_levels in range(2, 20):
    fig, ax = plt.subplots()
    im = ax.contourf(x, y, data, norm=LogNorm(), levels=n_levels)
    cbar = fig.colorbar(im, ax=ax)
    n_ticks = len(cbar.ax.get_yticklabels())
    print(f"Requested max {n_levels + 1}, got {n_ticks - 1}")

Gives on current main:

Requested max 3, got 7
Requested max 4, got 7
Requested max 5, got 7
Requested max 6, got 7
Requested max 7, got 7
Requested max 8, got 7
Requested max 9, got 7
Requested max 10, got 7
Requested max 11, got 7
Requested max 12, got 7
Requested max 13, got 7
Requested max 14, got 7
Requested max 15, got 7
Requested max 16, got 7
Requested max 17, got 7
Requested max 18, got 7
Requested max 19, got 7
Requested max 20, got 7

and with this PR:

Requested max 3, got 3
Requested max 4, got 4
Requested max 5, got 4
Requested max 6, got 4
Requested max 7, got 7
Requested max 8, got 7
Requested max 9, got 7
Requested max 10, got 7
Requested max 11, got 7
Requested max 12, got 7
Requested max 13, got 7
Requested max 14, got 7
Requested max 15, got 7
Requested max 16, got 7
Requested max 17, got 7
Requested max 18, got 7
Requested max 19, got 7
Requested max 20, got 7

PR checklist

@dstansby dstansby mentioned this pull request Dec 28, 2023
Closed
@dstansby dstansby marked this pull request as ready for review December 28, 2023 21:13
ksunden
ksunden reviewed Jan 9, 2024
View reviewed changes
@ebarcelosf ebarcelosf mentioned this pull request Nov 13, 2024
Closed
5 tasks
@dstansby dstansby force-pushed the log-contour-levels branch from f495d8d to 9cd6171 Compare March 24, 2025 21:49
@dstansby dstansby requested a review from ksunden March 27, 2025 09:31
@dstansby dstansby force-pushed the log-contour-levels branch from e908660 to 042e55a Compare April 8, 2025 08:51
timhoffm
timhoffm reviewed Apr 8, 2025
View reviewed changes
timhoffm
timhoffm reviewed Apr 8, 2025
View reviewed changes
@dstansby
Copy link
Copy Markdown
Member Author

dstansby commented Apr 9, 2025

Thanks - as well as applying your suggestions, I also factored out the logic to set the locator if it's not already set. This allows the long docstrings to be split up, and makes it explicit in _process_contour_level_args where the default locator is being set, instead of hiding it in _autolev.

timhoffm
timhoffm approved these changes Apr 9, 2025
View reviewed changes
lib/matplotlib/contour.py
Comment on lines 1028 to 1029
if args:
# Set if levels manually provided
levels_arg = args[0]
Copy link
Copy Markdown
Member

@timhoffm timhoffm Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This could still be made clearer / the current code is still convoluted (but possibly better as a followup PR to not make too many changes and complicate review)

args is not the original args but stripped from leading [X, Y], Z. Effectively it can only be empty or contain a single element that contains levels passed positionally (which is supported, but a bit cumbersome to write out as a signature because the optional [X, Y] require to absorb all positional parameters in *args and we therefore have to reunite a possible positional level from args with the possible kwarg level.

The current implementation also has the surprising side effect that you can do contour(Z, levels1, levels=levels2) and the positional levels1 is sliently ignored.

The only use of args here is to pass on a possible positional level. We can push the logic out of the function and unite the positional and kwarg paths of levels in the caller _contour_args - where it belongs logically.

I think eventually we should also make levels kw-only. contour(Z, 5) or contour(X, Y, Z, [-1, 0, 1]) are not very readable anyway.

QuLogic
QuLogic reviewed Apr 16, 2025
View reviewed changes
@dstansby dstansby force-pushed the log-contour-levels branch from cb2a823 to 8c0775e Compare April 30, 2025 09:28
@dstansby dstansby requested a review from QuLogic April 30, 2025 09:28
@dstansby dstansby added this to the v3.11.0 milestone Apr 30, 2025
@dstansby
Copy link
Copy Markdown
Member Author

dstansby commented Apr 30, 2025

I've milestoned this as 3.11 because it changes longstanding behaviour (even if it is a bugfix)

@dstansby dstansby merged commit 7fd4c67 into matplotlib:main Apr 30, 2025
40 checks passed
@dstansby dstansby deleted the log-contour-levels branch July 13, 2025 09:05
@lucyleeow lucyleeow mentioned this pull request Dec 15, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuLogic QuLogic QuLogic approved these changes

@timhoffm timhoffm timhoffm approved these changes

@ksunden ksunden Awaiting requested review from ksunden

Assignees

No one assigned

Labels

topic: contour

Projects

None yet

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@dstansby @QuLogic @ksunden @timhoffm