NCAR / geocat-viz

GeoCAT-viz contains tools to help plot geoscience data, including convenience and plotting functions that are used to facilitate plotting geosciences data with Matplotlib, Cartopy, and other visualization packages.
https://geocat-viz.readthedocs.io
Other
48 stars 19 forks source link

Remove matplotlib 3.8+ pin #160

Closed kafitzgerald closed 11 months ago

kafitzgerald commented 1 year ago

Describe the bug The plot_contour_labels usage example is not working properly with matplotlib 3.8+.

Most of this was because of issues with matplotlib and cartopy many of which have now been addressed, but few label locations are impacted by matplotlib/matplotlib#27333. Particularly for this usage example I think we should still just remove this pin and thin the number of labels (including the problematic ones).

You can see the problems here:

File ~/miniconda3/envs/geocat/lib/python3.11/site-packages/matplotlib/axes/_axes.py:6550, in Axes.clabel(self, CS, levels, kwargs) 6532 def clabel(self, CS, levels=None, kwargs): 6533 """ 6534 Label a contour plot. 6535 (...) 6548 All other parameters are documented in ~.ContourLabeler.clabel. 6549 """ -> 6550 return CS.clabel(levels, **kwargs)

File ~/miniconda3/envs/geocat/lib/python3.11/site-packages/matplotlib/contour.py:210, in ContourLabeler.clabel(self, levels, fontsize, inline, inline_spacing, fmt, colors, use_clabeltext, manual, rightside_up, zorder) 208 if np.iterable(manual): 209 for x, y in manual: --> 210 self.add_label_near(x, y, inline, inline_spacing) 211 elif manual: 212 print('Select label locations manually using first mouse button.')

File ~/miniconda3/envs/geocat/lib/python3.11/site-packages/matplotlib/contour.py:590, in ContourLabeler.add_label_near(self, x, y, inline, inline_spacing, transform) 587 label_width = self._get_nth_label_width(level) 588 rotation, path = self._split_path_and_get_label_rotation( 589 path, idx_vtx_min, proj, label_width, inline_spacing) --> 590 self.add_label(*proj, rotation, self.labelLevelList[idx_level_min], 591 self.labelCValueList[idx_level_min]) 593 if inline: 594 self._paths[idx_level_min] = path

IndexError: list index out of range



Likely related to these changes:
* https://matplotlib.org/stable/api/prev_api_changes/api_changes_3.8.0.html#contourset-is-now-a-single-collection
* https://github.com/matplotlib/matplotlib/pull/25247

Related issues over on cartopy:
* https://github.com/SciTools/cartopy/issues/2207
* https://github.com/SciTools/cartopy/issues/2224

Where I'm at in sorting this out:
* Have tested with the latest matplotlib and cartopy and am seeing the problems (fine w/ latest matplotlib 3.7)
* Messed around a bit with the plot_contour_labels example and it seems only some label locations trigger the issue (generally those closest to the edge of the contour / projected space)
* Similarly the NCL_coneff_8.py example on fails for certain labels
* Unfortunately sorting this out will likely require a deeper dive in matplotlib / cartopy than I have time for at the moment

I think the should throw in a version pin for the time being.

Side note:
* We could also check out what cartopy is doing for testing.  
kafitzgerald commented 1 year ago

Actually, I think this may be a bug in matplotlib. Something doesn't seem to be working properly in the code that finds the nearby contours to label.

Unfortunately we don't get an error in the plot_contour_labels usage example like we do in geocat-examples, but the usage example is also impacted. See here: https://geocat-viz.readthedocs.io/en/latest/examples/plot_contour_labels.html

I think we should probably pin to <3.8 and I'll report something over on the matplotlib repo once I have a little better of an idea what's going on.

kafitzgerald commented 11 months ago

Ok, it looks like this is more than one issue though some have been resolved by fixes upstream.

The remaining (after updating matplotlib and cartopy) spurious lines in the plot_contour_labels usage example are occurring because of one remaining bug in matplotlib. In particular, these three points are problematic:

(-58.78, 67.05) (-102.69, 73.62)
(-78.02, 52.22)

The remaining labels still overlap a bit more than they used to, but I think we should probably just thin out the number of labeled contour locations for the usage example.