Skip to content

[Bug]: Secondary axis does not support Transform as functions #25691

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

Closed
QuLogic opened this issue Apr 15, 2023 · 2 comments · Fixed by #27267
Closed

[Bug]: Secondary axis does not support Transform as functions #25691

QuLogic opened this issue Apr 15, 2023 · 2 comments · Fixed by #27267

Comments

@QuLogic
Copy link
Member

QuLogic commented Apr 15, 2023

Bug summary

The documentation for secondary_[xy]axis claims to accept a Transform for the functions parameter.

However, the checks in set_functions do not accept Transform:

if (isinstance(functions, tuple) and len(functions) == 2 and
callable(functions[0]) and callable(functions[1])):
# make an arbitrary convert from a two-tuple of functions
# forward and inverse.
self._functions = functions
elif functions is None:
self._functions = (lambda x: x, lambda x: x)
else:
raise ValueError('functions argument of secondary axes '
'must be a two-tuple of callable functions '
'with the first function being the transform '
'and the second being the inverse')

and any usage of the internal attribute seems to assume a tuple of callables:

set_scale('functionlog' if pscale == 'log' else 'function',
functions=self._functions[::-1])

lims = self._functions[0](np.array(lims))

Both this documentation and the checks have been in the code since it was added in #11859.

Code for reproduction

import matplotlib.pyplot as plt
from matplotlib.transforms import IdentityTransform

fig, ax = plt.subplots()
secax = ax.secondary_xaxis('top', functions=IdentityTransform)
# OR:
secax = ax.secondary_xaxis('top', functions=IdentityTransform())
# OR:
secax = ax.secondary_xaxis('top', functions=(IdentityTransform, IdentityTransform))
# OR:
secax = ax.secondary_xaxis('top', functions=(IdentityTransform(), IdentityTransform()))

Actual outcome

With a single transform (class or object), or a tuple of transform objects, it fails in the setter:

Traceback (most recent call last):
  File "galleries/examples/subplots_axes_and_figures/secondary_axis.py", line 40, in <module>
    secax = ax.secondary_xaxis('top', functions=IdentityTransform)
  File "lib/matplotlib/axes/_axes.py", line 586, in secondary_xaxis
    secondary_ax = SecondaryAxis(self, 'x', location, functions,
  File "lib/matplotlib/axes/_secondary_axes.py", line 42, in __init__
    self.set_functions(functions)
  File "lib/matplotlib/axes/_secondary_axes.py", line 155, in set_functions
    raise ValueError('functions argument of secondary axes '
ValueError: functions argument of secondary axes must be a two-tuple of callable functions with the first function being the transform and the second being the inverse

With a tuple of transform classes, the setter allows it, but it fails when drawing:

Traceback (most recent call last):
  File "lib/matplotlib/backends/backend_qt.py", line 461, in _draw_idle
    self.draw()
  File "lib/matplotlib/backends/backend_agg.py", line 401, in draw
    self.figure.draw(self.renderer)
  File "lib/matplotlib/artist.py", line 95, in draw_wrapper
    result = draw(artist, renderer, *args, **kwargs)
  File "lib/matplotlib/artist.py", line 72, in draw_wrapper
    return draw(artist, renderer)
  File "lib/matplotlib/figure.py", line 3115, in draw
    artists = self._get_draw_artists(renderer)
  File "lib/matplotlib/figure.py", line 225, in _get_draw_artists
    child.apply_aspect(
  File "lib/matplotlib/axes/_secondary_axes.py", line 120, in apply_aspect
    self._set_lims()
  File "lib/matplotlib/axes/_secondary_axes.py", line 216, in _set_lims
    lims = self._functions[0](np.array(lims))
  File "lib/matplotlib/transforms.py", line 1759, in __init__
    super().__init__(*args, **kwargs)
  File "lib/matplotlib/transforms.py", line 125, in __init__
    self._shorthand_name = shorthand_name or ''
ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

Expected outcome

At least one of the options works, or it should not be documented as accepted.

Additional information

I noticed this oddity in the types while reviewing #25224, but not sure what we intend here.

Operating system

No response

Matplotlib Version

3f4d4c1

Matplotlib Backend

No response

Python version

No response

Jupyter version

No response

Installation

git checkout

@jklymak
Copy link
Member

jklymak commented Apr 15, 2023

That seems like an oversight, or something that got lost in the shuffle of adding a forward and inverse.

I don't think we should allow a tuple of transforms - one of the advantages of using a transform is that it has an inverse?

@QuLogic
Copy link
Member Author

QuLogic commented May 5, 2023

I don't think we should allow a tuple of transforms - one of the advantages of using a transform is that it has an inverse?

I don't think it's necessary; it's just something I tried to get past the checks and see if the internals were fine with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants