-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[Bug]: plt.clabel raises exception at very low DPI: ValueError: 'codes' must be a 1D list or array with the same length of 'vertices'. Your vertices have shape (2, 2) but your codes have shape (1,)
#26971
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Require matplotlib<3.8.0 due to matplotlib/matplotlib#26971.
One odd thing here is that pdf is a vector format and all of the items are drawn as vectors here, so it is mildly unexpected to me that the dpi affects this at all. Now, My guess is that there is logic on how big to make the break in the contour that is based on the dpi to some extent, and that the vector formats use the same computation to make sure that placement of text is the same for them as it is for e.g. png. The error actually occurs on the clabel call itself, so before the When I use the pdf backend, it does not error (it also does not error at 5 dpi, for the record, and at 3 dpi gets a different error that it can't set the font size). I will also note that the positioning of the clabel text does differ between using pdf backend when calling That said, we are generating invalid paths which should not be possible. This does come back to #25247, which changed the inheritance of ContourSet The @anntzer, thoughts? |
The following diff makes it so it at least does not error (debugging prints left in for right now): diff --git a/lib/matplotlib/contour.py b/lib/matplotlib/contour.py
index efea024dc1..a1579b77a5 100644
--- a/lib/matplotlib/contour.py
+++ b/lib/matplotlib/contour.py
@@ -409,6 +409,9 @@ class ContourLabeler:
# Get indices near points of interest; use -1 as out of bounds marker.
i0, i1 = np.interp(target_cpls, path_cpls, range(len(path_cpls)),
left=-1, right=-1)
+ print(f"pre-rounding {i0=} {i1=}")
+ if i0 < i1:
+ i1, i0 = i0, i1
i0 = math.floor(i0)
i1 = math.ceil(i1)
(x0, x1), (y0, y1) = interp_vec(target_cpls, path_cpls, cc_xys)
@@ -418,9 +421,11 @@ class ContourLabeler:
new_code_blocks = []
if is_closed_path:
if i0 != -1 and i1 != -1:
+ print(f"Closed path {i0=} {i1=}")
new_xy_blocks.extend([[(x1, y1)], cc_xys[i1:i0+1], [(x0, y0)]])
new_code_blocks.extend([[Path.MOVETO], [Path.LINETO] * (i0 + 2 - i1)])
else:
+ print(f"Open path {i0=} {i1=}")
if i0 != -1:
new_xy_blocks.extend([cc_xys[:i0 + 1], [(x0, y0)]])
new_code_blocks.extend([[Path.MOVETO], [Path.LINETO] * (i0 + 1)]) Essentially what was happening was that Ensuring that i0 is greater fixes this. The output is not quite identical to what you had previously, though quite close:
@anntzer do you think this is the correct fix? perhaps the swap should happen after the floor/ceiling calls (I lean towards before, but wasn't 100% sure)? |
From a quick look I think this basically occurs if diff --git i/lib/matplotlib/contour.py w/lib/matplotlib/contour.py
index efea024dc1..9aa4c092ca 100644
--- i/lib/matplotlib/contour.py
+++ w/lib/matplotlib/contour.py
@@ -404,6 +404,9 @@ class ContourLabeler:
if self.rightside_up: # Fix angle so text is never upside-down
angle = (angle + 90) % 180 - 90
+ if is_closed_path and lw + 2 * spacing > path_cpls[-1] - path_cpls[0]:
+ return angle, Path(np.empty((0, 2)))
+
target_cpls += [-spacing, +spacing] # Expand range by spacing.
# Get indices near points of interest; use -1 as out of bounds marker. As a side point, this whole "remove parts of the contour path to draw text over it" logic is really terrible; we should really have something like knockout groups in matplotlib (i.e. in a temporary buffer, render the (unbroken) contour and draw the text over it with a surrounding box that resets the buffer to transparent, and then draw the temporary buffer to the main one). But that's a much bigger ask, of course. |
diff --git a/lib/matplotlib/contour.py b/lib/matplotlib/contour.py
index 941ebf7aed..ccbcc075f7 100644
--- a/lib/matplotlib/contour.py
+++ b/lib/matplotlib/contour.py
@@ -419,7 +419,7 @@ class ContourLabeler:
if is_closed_path:
if i0 != -1 and i1 != -1:
new_xy_blocks.extend([[(x1, y1)], cc_xys[i1:i0+1], [(x0, y0)]])
- new_code_blocks.extend([[Path.MOVETO], [Path.LINETO] * (i0 + 2 - i1)])
+ new_code_blocks.extend([[Path.MOVETO], [Path.LINETO] * (len(cc_xys[i1:i0+1]) + 1)])
else:
if i0 != -1:
new_xy_blocks.extend([cc_xys[:i0 + 1], [(x0, y0)]]) This appears to give identical behavior to 3.7.3 (the file linked in the original post), though I think that removing is "more correct" (if a bit odd looking as there is a label with no contour at all) as I do think this is the result of the expansion from the label actually eliminating all points, just this adds two of them back. I think flipping i0 and i1 (my first suggestion) has the same endpoints, but brings back the point in between, whereas this (and pre-3.8) ignore that point between the two endpoints it sees. I do agree that this path manipulation/breaking is brittle at the very least, but that a more comprehensive solution is out of scope for this particular issue. |
The last patch seems fine to me as a stopgap. |
Bug summary
This worked up to and including Matplotlib 3.7.3 and broke in Matplotlib 3.8.0.
Data files for sample code below:
x.txt
y.txt
z.txt
Code for reproduction
Actual outcome
Expected outcome
test.pdf
Additional information
I discovered this regression in the CI pipeline from one of my own projects, in a test that sets DPI to an extremely low value for performance purposes. https://git.ligo.org/lscsoft/ligo.skymap/-/jobs/2961694
Operating system
macOS
Matplotlib Version
3.8.0
Matplotlib Backend
MacOSX
Python version
3.11.5
Jupyter version
N/A
Installation
pip
The text was updated successfully, but these errors were encountered: