Let as_mpl_selector forward **kwargs to *Selector

[dhomeier]

The docstring for the as_mpl_selector methods states that any additional **kwargs can be passed to the respective matplotlib selector instances, but currently they are in fact discarded. This PR passes them through.
In addition I have added the option to override the default rectprops by kwarg – which otherwise might conflict with the argument specified by the user, although it's perhaps to be discussed whether these should always be set by region.visual.

And I think there is a point to have some parameters still set to different defaults from mpl, if not explicitly set by the user, e.g. {'edgecolor': 'black', 'facecolor': 'none'} is certainly the more reasonable default here.

Also todo tests (?); should any/which additional parameters, like drag_from_anywhere, be specifically mentioned in the docstring?

[keflavich]

Yes, let's mention drag_from_anywhere explicitly in the docstring.

[dhomeier]

Marking this as ready for review now, although there could be one or two tests for passing other kwargs added, + the docstring for drag_from_anywhere.
This seems to solve one part of #415; but matplotlib versions < 3.3 that do not have on_select need to be taken care of – either adding our own replacement for the convenience function, or simply skipping those 3 tests.

[dhomeier]

I should note that the second part of test_mpl_selector_drag is supposed to show that move does not work outside the centre anchor; however it behaves exactly the same with drag_from_anywhere=False as (it is supposed to) with drag_from_anywhere=True. Interactively this works as expected; no idea if then on_select or MouseEvent still behave in some way differently from clicking the real mouse button.

Also not sure if the version check is worth importing packaging.version; it could still relatively easily be done on _mpl_version.split('.'), but it would be nice to have a somewhat more centralised mpl_version control.
Related to this, the new kwargs tests would also need version conditionals, since some of the parameters have been renamed as well in 3.4 or 3.5.

Lastly, the rectprops/props check still allows the user to specify the wrong one; might say this is their responsibility to use the correct form for their matplotlib version, but since passing both kwargs has very weird side effects (e.g. edgecolor becoming facecolor; other parameters getting lost entirely), I am wondering if the user args should get an additional sanity-check.

[dhomeier]

I've pulled the Matplotlib checks up one level into shapes/utils.py to make an integer MPL_VERSION ([major][minor]) available. Might be useful even at the top level, but I haven't found a good place to put it, and outside shapes there is not much reference to HAS_MATPLOTLIB and I think none to its version.

I've also included a fix to the Mpl intersphinx setup for testing: it seems the base URL has been moved around 3.5 from https://matplotlib.org/ to https://matplotlib.org/stable/ ; if you compare the link to matplotlib.widgets.EllipseSelector in
https://astropy-regions.readthedocs.io/en/stable/api/regions.EllipsePixelRegion.html
to
https://astropy-regions--392.org.readthedocs.build/en/392/api/regions.EllipsePixelRegion.html
the former is pointing to the old URL, being redirected, but losing the section reference in the process.
Seems to work with the changed URL, so this should probably be repaired upstream in astropy docs/conf.py, but we could keep it here as a temporary fix.

[keflavich]

lgtm; I haven't run this locally but it looks good.

[dhomeier]

Changed to AttributeError (have to remember to sync this in the Polygon selector PR as well).
I've reverted the intersphinx path fix, since it is now in Astropy 5.0.1.

[larrybradley]

Is this ready for a final review?

[dhomeier]

Is this ready for a final review?

Yes, should now - there was a call from the mpl_selector docs that only worked with mpl dev.

[larrybradley]

Thanks, @dhomeier! This looks great. I had just a minor comment about checking the matplotlib version. I'll be glad to stop getting emails about the failing devdeps CI test!

[larrybradley]

Tests are now failing after the updated matplotlib version check.

[larrybradley]

@dhomeier There are merge conflicts. Can you please rebase?

[dhomeier]

Doh, was caught right in between by 62eb153 (from the test I thought visual was supposed to not be copied over)!

[larrybradley]

Thanks!