Skip to content

FIX: Deprecate using clabel() with filled contours#31347

Open
timhoffm wants to merge 1 commit intomatplotlib:mainfrom
timhoffm:deprecate-contourf-clabel
Open

FIX: Deprecate using clabel() with filled contours#31347
timhoffm wants to merge 1 commit intomatplotlib:mainfrom
timhoffm:deprecate-contourf-clabel

Conversation

@timhoffm
Copy link
Member

@timhoffm timhoffm commented Mar 22, 2026

It's never been intended to be used that way and may cause inconsistent plots. Closes #31344.

@timhoffm timhoffm force-pushed the deprecate-contourf-clabel branch from 3b2af05 to a5b4b7f Compare March 22, 2026 10:40
rcomer
rcomer reviewed Mar 22, 2026
View reviewed changes
Copy link
Member

@rcomer rcomer left a comment

Choose a reason for hiding this comment

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

The failing test was added at #16329. I assume the contourf part was just done for completeness since the issue only seems concerned with labels on a contour plot.

@timhoffm timhoffm mentioned this pull request Mar 22, 2026
Open
@timhoffm timhoffm force-pushed the deprecate-contourf-clabel branch from a5b4b7f to 46baee2 Compare March 22, 2026 11:45
@timhoffm timhoffm marked this pull request as draft March 22, 2026 12:39
@timhoffm
Copy link
Member Author

timhoffm commented Mar 22, 2026

Still need to fix test failures.

@rcomer
Copy link
Member

rcomer commented Mar 22, 2026

Should there also be a deprecated directive on the clabel docstring? Maybe not if this was never supposed to work.

@timhoffm
Copy link
Member Author

timhoffm commented Mar 23, 2026 •
edited
Loading

IMHO we do not need to add a deprecation message to the docstring. Since this resulted in broken plots, I was on the edge of directly raising to reduce effort. Doing the deprecation is the more defensive approach of first notifying people that their plot is broken before actually raising.

The deprecation is on the technical level (disallow the code path). Semantically, it was never intended or did work. So as far as the docstring is concerned, a clarification is sufficient.

@timhoffm
FIX: Deprecate using clabel() with filled contours
169a730 
It's never been intended to be used that way and may cause inconsistent
plots. Closes matplotlib#31344.
@timhoffm timhoffm force-pushed the deprecate-contourf-clabel branch from 46baee2 to 169a730 Compare March 23, 2026 02:30
@timhoffm
Copy link
Member Author

timhoffm commented Mar 23, 2026

I agree with #31347 (review) that the cs_filled test was likely only added for completeness and without realization that clabel does not work properly with contourf.

@timhoffm timhoffm marked this pull request as ready for review March 23, 2026 02:32
themavik

This comment was marked as spam.

Sign in to view

@rcomer

This comment was marked as outdated.

Sign in to view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rcomer rcomer rcomer approved these changes

+1 more reviewer

@themavik themavik themavik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

topic: contour

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Adding contour labels affects the shape of filled contours

3 participants

@timhoffm @rcomer @themavik