plot_motion does not accept axes or ax argument

[JoeZiminski]

The widgets.motion.plot_motion() function asserts that ax and axes is None here but the documentation says these should be accepted inputs.

Is there a reason for this? Happy to open a PR to make the function support ax / axes arguments if not.

Also the latest docs have a number of references to si.load_motion_info and si.plot_motion but it appears these have since moved, also happy to open PR ASAP but leaving here as don't want to forget.

[zm711]

Not a clue. @alejoe91 or @samuelgarcia would have to weigh in on this. Based on the code it looks like it generates its own sub-ax so maybe they didn't want the user to supply an ax since it needs to attach ax0, ax1 and ax2 to the figure.

[JoeZiminski]

Yes indeed this makes sense, it works well with figure argument. I think in this case just a message needs to be added to assert and docs updated. Will open a PR on this if all agree.

[zm711]

As far as assert messaging I don't think anyone would complain about that :) But we can definitely wait for the others to weigh in.

[samuelgarcia]

Hi
In si, some matplotlib plotting method accept ax or axes externally, which is convinient to construct summary figures for instance. When multiple axes it is more tedious to handle it so we put I put this assert at some places which was more easy. Unfortunatly the doc is generic about this backend_kwargs.

[zm711]

So I think the fix would be like what @JoeZiminski said. Add better assert messaging so the user knows this is one function that can't accept external ax/axes. Good for you @samuelgarcia ?

[samuelgarcia]

yes yes of course (or making the code accept axes)

[JoeZiminski]

Fixed in #3068

[JoeZiminski]

Well, kind of, it still says in the documentations it is accepted but this is hard to fix, the assert message is clear so now it is immediately clear what to do, so fixed-enough.