Fix Axes clearing with Matplotlib 3.6+ #2071
Conversation
lib/cartopy/mpl/geoaxes.py
Outdated
| if mpl.__version__ >= '3.6': | ||
| def clear(self): | ||
| """Clear the current Axes and add boundary lines.""" | ||
| result = super().clear() | ||
| self._clear() | ||
| return result | ||
| else: | ||
| def cla(self): | ||
| """Clear the current Axes and add boundary lines.""" | ||
| result = super().cla() | ||
| self._clear() | ||
| return result |
There was a problem hiding this comment.
Rather than explicit version gating, what about just defining clear ourselves even for older versions of MPL.
| if mpl.__version__ >= '3.6': | |
| def clear(self): | |
| """Clear the current Axes and add boundary lines.""" | |
| result = super().clear() | |
| self._clear() | |
| return result | |
| else: | |
| def cla(self): | |
| """Clear the current Axes and add boundary lines.""" | |
| result = super().cla() | |
| self._clear() | |
| return result | |
| def clear(self): | |
| """Clear the current Axes and add boundary lines.""" | |
| result = getattr(super(), "clear", super().cla)() | |
| self._clear() | |
| return result | |
| def cla(self): | |
| """Clear the current Axes and add boundary lines.""" | |
| return self.clear() |
There was a problem hiding this comment.
Axes.clear always existed though, so I don't think that getattr fallback will work.
There was a problem hiding this comment.
Yep I agree, I don't think I realized that it was the coming back down calls that were being missed. It might be helpful to put a quick comment about removing this once we only require MPL 3.6.
|
This should be fixed upstream. |
|
@tacaswell Are you saying upstream needs to put in a fix? Or it already has? I can't find a relevant PR. |
|
That is a prescriptive "should" as I think this is a release blocker for Matplotlib. I do not fully understand the issue yet, but I do not think we intended to remove support for either |
|
@tacaswell That's what I thought, but wasn't sure. So I just finally made sense of it too. The problem is that matplotlib itself changed all calls to use |
|
Ah, so we need check in |
|
@tacaswell That makes sense to me. @QuLogic ? Would definitely affect how we proceed here. |
|
I've put in matplotlib/matplotlib#23735 upstream. I'm not sure that we actually want to warn about it. However, I think we still want to move forward this this PR, so that we (eventually when Matplotlib 3.6 is the minimum required) are using the new term. |
lib/cartopy/mpl/geoaxes.py
Outdated
| if mpl.__version__ >= '3.6': | ||
| def clear(self): | ||
| """Clear the current Axes and add boundary lines.""" | ||
| result = super().clear() | ||
| self._clear() | ||
| return result | ||
| else: | ||
| def cla(self): | ||
| """Clear the current Axes and add boundary lines.""" | ||
| result = super().cla() | ||
| self._clear() | ||
| return result |
There was a problem hiding this comment.
Yep I agree, I don't think I realized that it was the coming back down calls that were being missed. It might be helpful to put a quick comment about removing this once we only require MPL 3.6.
|
I definitely agree we should go ahead and future proof, I just wasn't sure if the form of the fix here would change based on what's merged for matplotlib 3.6. |
There was a problem hiding this comment.
I just checked out both development branches to test these updates out, and this is definitely close. On Cartopy main I get the expected PendingDeprecationWarning, but on this branch there are a bunch of failures. This is due to some more naming conflicts now that Matplotlib added a private _clear() method that is called and you're also redefining that here. The easiest fix is probably to update the name here to something explicit and only for Cartopy.
lib/cartopy/mpl/geoaxes.py
Outdated
| def cla(self): | ||
| """Clear the current axes and adds boundary lines.""" | ||
| result = super().cla() | ||
| def _clear(self): |
There was a problem hiding this comment.
| def _clear(self): | |
| def _cartopy_clear(self): |
and update the calls below.
There was a problem hiding this comment.
we changed to __clear upstream to get the name mangling, but this is probably a good idea anyway!
|
I just tested this out locally with rc2 and all worked well for me. |
Rationale
Matplotlib 3.6 expired the
cla/clearalias, soGeoAxeswere not being correctly cleared with Matplotlib 3.6.Implications
Plots don't get spectacularly broken.