Skip to content

Commit

Permalink
Relax conditions for warning on updating converters
Browse files Browse the repository at this point in the history
Allow sub/superclass without warning.

This was motivated by Pandas, as they have several instances of the same class in the registry.
We also had this internally, though resolved that by using the same instance for multiple registry
entries rather than by relaxing the warning. This becomes impractical when dealing with downstream.

It may have been sufficient to allow only identical types, but to be safe, decided to allow both
sub- and superclass relationships.
The purpose of the warning was that it gets overwritten without heeding prior data provided which
would break in unexpected ways.
The most common case for this is when one converter class operates on multiple data types, but
separate instances of the class are in the registry. So while the previous data is compatible the
equality check still failed. Requiring downstream to implement equality checks seems impractical.

Additionally used both `is` and `==` for the earlier short circuit, which should in most cases be
identical operations, but just in case.
  • Loading branch information
ksunden committed Nov 18, 2024
1 parent 183b04f commit 530ac3d
Showing 1 changed file with 6 additions and 2 deletions.
8 changes: 6 additions & 2 deletions lib/matplotlib/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1842,11 +1842,15 @@ def set_converter(self, converter):
self._converter_is_explicit = True

def _set_converter(self, converter):
if self._converter == converter:
if self._converter is converter or self._converter == converter:
return
if self._converter_is_explicit:
raise RuntimeError("Axis already has an explicit converter set")
elif self._converter is not None:
elif (
self._converter is not None and
not isinstance(converter, type(self._converter)) and
not isinstance(self._converter, type(converter))
):
_api.warn_external(
"This axis already has a converter set and "
"is updating to a potentially incompatible converter"
Expand Down

0 comments on commit 530ac3d

Please sign in to comment.