Skip to content

Doc: Clarify default levels behavior in contour/contourf #31016

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.

Sign up for GitHub

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

Jump to bottom
Merged
timhoffm merged 9 commits into from
Feb 2, 2026
Merged

Doc: Clarify default levels behavior in contour/contourf #31016

timhoffm merged 9 commits into from
Feb 2, 2026

Conversation

@sanrishi
Copy link
Contributor

@sanrishi sanrishi commented Jan 22, 2026 •
edited
Loading

PR summary

Problem

The documentation for the levels parameter was ambiguous regarding the exact number of resulting lines versus filled regions. It stated "no more than n+1" levels, but MaxNLocator often produces n+2 boundaries (e.g., input n=7 results in 9 boundaries).

Solution

I have updated the docstring to define levels specifically as "contour boundaries". This resolves the ambiguity because:

  • contour() draws lines at these boundaries.
  • contourf() fills the regions between these boundaries.

This definition remains accurate even when extend is used (which adds extra regions beyond the boundaries) or when MaxNLocator adjusts the tick count.

Verification

I verified the behavior locally using a test script to compare MaxNLocator output against the actual generated lines and collections.

Findings for levels=n:

  • MaxNLocator produces up to n+2 boundaries.
  • contour produces n+2 lines.
  • contourf produces n+1 filled regions (inherently N_boundaries - 1).

Visual Proof:
I generated the following comparison to demonstrate that filled regions exist between the lines, confirming that the "Boundaries" terminology is the correct source of truth.

contour_vs_contourf_comparison ## PR checklist

@sanrishi
Doc: Clarify default levels behavior in contour/contourf
44c1dd4 
Explicitly state that the default levels corresponds to n=7 and clarifies that int input targets n+1 levels but may produce n+2.
@sanrishi sanrishi marked this pull request as draft January 22, 2026 13:39
@sanrishi sanrishi force-pushed the fix-contour-levels-doc branch from 3c6bd7d to adbf65f Compare January 22, 2026 14:32
@sanrishi sanrishi marked this pull request as ready for review January 22, 2026 14:58
@sanrishi sanrishi force-pushed the fix-contour-levels-doc branch from adbf65f to e64f5eb Compare January 22, 2026 15:17
@sanrishi sanrishi mentioned this pull request Jan 23, 2026
Open
@sanrishi
Copy link
Contributor Author

sanrishi commented Jan 27, 2026

@timhoffm can you please take a look on this as well
It's been so many days I opened this

@sanrishi sanrishi mentioned this pull request Jan 27, 2026
Closed
@story645
Copy link
Member

story645 commented Jan 27, 2026

I prefer the wording in #31013 and it has precedence since it was opened first. I just haven't had a chance to check if the build errors are real.

@sanrishi
Copy link
Contributor Author

sanrishi commented Jan 27, 2026

@story645 since #31013 is not mentioned his pr on issue I worked on it for hours

I updated the code so it directly addresses @lucyleeow's question about needing the exact number while maintaining the "reasonable" language @timhoffm suggested.
#31013 approach is wrong

If an int *n*, use `~matplotlib.ticker.MaxNLocator`, which tries
to automatically choose no more than *n+1* "nice" contour levels
  • User sets levels=1
  • Code does MaxNLocator(1+1) = MaxNLocator(2)
  • Max locator produces up to 3 ticks/levels
  • So "no more than n+1" is FALSE - it's actually "up to n+2"

@sanrishi
Copy link
Contributor Author

sanrishi commented Jan 27, 2026

@story645 Thoughts on my reply ??

@story645
Copy link
Member

story645 commented Jan 28, 2026 •
edited
Loading

@story645 Thoughts on my reply ??

Please keep in mind that our project favors collaboration (contributing guide) and that the solution in #31013 was proposed before @lucyleeow's comment.

I won't have the bandwith to verify Lucy's correction for the next few days but it would be quite valuable if you'd do so by pulling up the code that does the computation, walking through it, and succinctly writing it up. Please also be thoughtful in your AI use - I'm not just feeding these instructions into a chat b/c the human verification is the important part.

@story645 story645 mentioned this pull request Jan 28, 2026
Merged
@sanrishi sanrishi closed this Jan 29, 2026
timhoffm
timhoffm reviewed Jan 29, 2026
View reviewed changes
lib/matplotlib/contour.py Outdated
Comment on lines 1489 to 1491
If an int *n*, use `~matplotlib.ticker.MaxNLocator`, which tries to
automatically choose no more than *n+2* "nice" contour levels
between the minimum and maximum numeric values of *Z*.
Copy link
Member

@timhoffm timhoffm Jan 29, 2026

Choose a reason for hiding this comment

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

This is still not correct, it uses MaxNLocator to choose no more that n+2 "boundaries". But that has to be translated to number of contour lines / regions. I think - but to be checked - for contour the number of lines is the number of boundaries, but for contourfthe number of filled regions is one more.

@sanrishi if you want, you are welcome to verify this and reopen with an update on this part of the docs.

Copy link
Contributor Author

@sanrishi sanrishi Feb 1, 2026

Choose a reason for hiding this comment

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

@timhoffm I have updated the docstring to use the "boundaries" definition and added the verification plots
Ready for review!

@sanrishi sanrishi reopened this Jan 29, 2026
@sanrishi sanrishi marked this pull request as draft January 29, 2026 11:43
@sanrishi
DOC: Clarify contour levels parameter as boundaries to fix ambiguity
1cd6255 
The previous documentation stated that `MaxNLocator` chooses "no more than n+1" levels, which was mathematically incorrect for the default case (n=7 produces 9 levels).

Additionally, the distinction between `contour` (lines) and `contourf` (filled regions) created confusion, as `contourf` produces one fewer region than the number of lines (unless `extend` is used).

This update redefines `levels` strictly as "contour boundaries." This terminology:
1. Is accurate for both functions (lines are at boundaries, regions are between them).
2. Remains correct regardless of the `extend` parameter.
3. Explicitly documents the default behavior for n=7.
@sanrishi sanrishi marked this pull request as ready for review February 1, 2026 11:40
@sanrishi sanrishi requested a review from timhoffm February 1, 2026 15:53
timhoffm
timhoffm reviewed Feb 2, 2026
View reviewed changes
@sanrishi @timhoffm
Update lib/matplotlib/contour.py
5ff02f0 
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
@sanrishi
Copy link
Contributor Author

sanrishi commented Feb 2, 2026

@timhoffm timeout is still happening

@timhoffm
Copy link
Member

timhoffm commented Feb 2, 2026 •
edited
Loading

Yes, I know. Timeouts are much rarer now, but still occur.

@sanrishi
Copy link
Contributor Author

sanrishi commented Feb 2, 2026

@timhoffm should we proceed to solve remaining timeout issue or this is acceptable?

@timhoffm
Copy link
Member

timhoffm commented Feb 2, 2026

Of course, fixing the timeouts remains highly desirable.

@timhoffm timhoffm added this to the v3.11.0 milestone Feb 2, 2026
@timhoffm timhoffm merged commit 51fbfc4 into matplotlib:main Feb 2, 2026
37 of 40 checks passed
@sanrishi sanrishi deleted the fix-contour-levels-doc branch February 2, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timhoffm timhoffm timhoffm approved these changes

Assignees

No one assigned

Labels

Documentation topic: contour

Projects

None yet

Milestone

v3.11.0

Development

Successfully merging this pull request may close these issues.

[Doc]: contour and contourf levels default not specified

4 participants

@sanrishi @story645 @timhoffm @melissawm