FIX: Guard against already-removed labels in ContourSet.remove()#31431
FIX: Guard against already-removed labels in ContourSet.remove()#31431buddy0452004 wants to merge 4 commits intomatplotlib:mainfrom
Conversation
rcomer
left a comment
rcomer
left a comment
There was a problem hiding this comment.
Thanks @buddy0452004, but I do not think you understood the root cause of why the failure happens. I think the explanation at #31404 (comment) is correct. The error shown in the issue states that we tried to remove something from a list that is not in the list. So we got hold of the list axes.texts without problem, but the labels were no longer in there.
lib/matplotlib/tests/test_contour.py
Outdated
|
|
||
| def test_contour_remove_after_label_removed(): | ||
| # Test that CS.remove() works even if labels were manually removed first | ||
| # Regression test for https://github.com/matplotlib/matplotlib/issues/31404 |
There was a problem hiding this comment.
We do not typically reference issues in test comments.
lib/matplotlib/tests/test_contour.py
Outdated
| x = np.linspace(-3.0, 3.0, 20) | ||
| y = np.linspace(-2.0, 2.0, 20) | ||
| X, Y = np.meshgrid(x, y) | ||
| Z = np.exp(-X**2 - Y**2) |
There was a problem hiding this comment.
For efficiency, try to use the simplest data input you can that still makes the test effective. Look at test_contourf_rasterize for an example.
lib/matplotlib/tests/test_contour.py
Outdated
|
|
||
|
|
||
|
|
||
| def test_contour_remove_after_label_removed(): |
There was a problem hiding this comment.
Since there is already a test_contour_remove further up the file, I would put this one right below it, to keep tests with similar purposes together.
lib/matplotlib/contour.py
Outdated
| for text in self.labelTexts: | ||
| text.remove() | ||
| for text in list(self.labelTexts): | ||
| if axes is not None and text in axes.texts: |
There was a problem hiding this comment.
Why do we check whether axes is None here?
|
@rcomer Thanks for the review! You're right about the root cause the issue is that the labels were already removed from Also:
|
|
I'm putting this on hold until we have clarified what is actually needed. #31404 (comment) |
4 participants
When contour labels are manually removed before calling CS.remove(), matplotlib still tries to remove them again internally, causing a ValueError.
The root cause is that super().remove() detaches the contour from the axes, setting self.axes to None. Any subsequent access to self.axes.texts then crashes. The fix saves the axes reference before calling super().remove(), then uses it safely with a None check.
Also adds a regression test for this case.
Closes #31404
AI Disclosure
No AI used.
PR Checklist