Add error message and tests for plot_motion.

[JoeZiminski]

This PR closes #2725 by adding an error message when ax or axes arguments are passed, these are backend_kwargs which are not supported for this function.

This is slightly suboptimal as the documentation still says they are accepted, but at least it is clear what to do when it does go wrong. I looked into making the function support passing axes but it would make things very complicated for (I don't think?) not too much benefit.

I also added tests for the function and made a couple of minor changes:

• Added a little more explanation to the docstrings..
• renamed the variables x and y to make slightly more descriptive.
• renamed x-axis label Times [s] to Time [s].

I had a couple of questions:

• The motion_lim is documented as Tuple or None but it actually expects an int. I guess this could be changed to accept a Tuple similar to depth_lim?
• at the moment the widget object .ax is set to an empty Axes object. Maybe it would be clearer to set this to None as it is unused?

Also a more general question, the matplotlib and scipy imports are not at the top of the file, and I see the tests do not expect scipy to be installed. I guess the logic is the same for matplotlib which is why it is not top of file? Personally I would add any 'core' python modules to the general dependencies and then there is less maintenance required to have to workaround users potentially not having them installed. I don't think anyone will have trouble installing matplotlib or scipy unlike heavier modules such as numba. However may there are other considerations (e.g. docker images?).

[alejoe91]

I had a couple of questions:

• The motion_lim is documented as Tuple or None but it actually expects an int. I guess this could be changed to accept a Tuple similar to depth_lim?

Yeah I think that sounds super reasonable

• at the moment the widget object .ax is set to an empty Axes object. Maybe it would be clearer to set this to None as it is unused?

Agreed

Also a more general question, the matplotlib and scipy imports are not at the top of the file, and I see the tests do not expect scipy to be installed. I guess the logic is the same for matplotlib which is why it is not top of file? Personally I would add any 'core' python modules to the general dependencies and then there is less maintenance required to have to workaround users potentially not having them installed. I don't think anyone will have trouble installing matplotlib or scipy unlike heavier modules such as numba. However may there are other considerations (e.g. docker images?).

The idea behind core VS full is that core can be used as a super lightweight module only to use the SI data model, e.g., from a third party package. Adding main Python modules to core could result in dependencies issues for these kind of 3rd party minimal usage.

[JoeZiminski]

Thanks @alejoe91! I've made the related changes, this is now ready for review.

Out of interest what kind of third-party apps need low-overhead installations? This would be useful for me to know in general when thinking about dependencies on other packages.

[JoeZiminski]

I actually think I got this wrong, it is displacement across binned depths, not channels? In general I can fix these docstring to better use the terminology from the drift correction tutorial.

[JoeZiminski]

Okay I think I have fixed it now, by replacing "per channel" with "per spatial bin".

[samuelgarcia]

Suggested change

	backend : Union[None, "matplotlib"]
	Determines the plotting backend. Only "matplotlib" currently supported.

This is automatically generated

[JoeZiminski]

Thanks @samuelgarcia, you mean it defaults to None -> "matplotlib" and so never needs to be set? If so, in can it be removed entirely as an argument tot his class? If it cannot be removed (e.g. to keep consistency with BaseWidget) I would suggest to add as docstring parameter anyway to avoid confusion.