-
-
Notifications
You must be signed in to change notification settings - Fork 58
Description
Opening this just to keep track of the task list outlined and partly completed in #317:
The Matplotlib widgets included in Matplotlib don't (yet) have everything we need, in particular they don't have the ability to support rotation. However, I'm going to investigate whether the Matplotlib developers would be happy to have this additional functionality. Also moving regions around is a bit obnoxious as it requires clicking on the center of the region. I'm going to see if I can patch it to allow the whole region to be clicked and dragged. But in any case, we can either improve the widgets in Matplotlib or fork it here if they won't accept improvements upstream (which I doubt).
@keflavich @larrybradley - what do you think about this approach and API?
TODOs if we agree on the approach:
- Use the region's visual properties for the selector
- Changelog entry
- Tests
- Expand approach to other region types
- Investigate how to deal with Sky regions and WCSAxes
- Improve Matplotlib selectors to support e.g. rotation (could potentially be done as part of another PR since we can just raise a NotImplementedError in the mean time)
The last point is under development in #390.
Re. allow the whole region to be clicked and dragged this is already implemented by holding space and seems to just work, but the matplotlib documentation feels a bit unclear about it. The modifier key is defined for _SelectorWidget in
https://github.com/matplotlib/matplotlib/blob/01fb7bb2d9a674a05f704fc50d36e0baa7d6079c/lib/matplotlib/widgets.py#L1798-L1799
but the docstring e.g. for RectangleSelector states
https://github.com/matplotlib/matplotlib/blob/01fb7bb2d9a674a05f704fc50d36e0baa7d6079c/lib/matplotlib/widgets.py#L2626-L2634
where possibly ' ' has simply been confused for None. Ping the matplotlib devs for clarification?
Additionally matplotlib/matplotlib#19657 has added an option to always enable this behaviour; currently this can be activated by
selector.drag_from_anywhere = Trueso all that remains to be done there would be to let region.as_mpl_selector pass this kwarg to the Selector on creation.
According to the docs this should already be done, but apparently **kwargs have got lost somewhere on the way to the *Selector call; I've set up a tentative fix in #392.