search

astropy / regions

Astropy package for region handling
https://astropy-regions.readthedocs.io
BSD 3-Clause "New" or "Revised" License
61 stars 55 forks source link

Outstanding work on interactive regions functionality #391

Open dhomeier opened 3 years ago

dhomeier commented 3 years ago

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:

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 https://github.com/matplotlib/matplotlib/pull/19657 has added an option to always enable this behaviour; currently this can be activated by

selector.drag_from_anywhere = True

so 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.

keflavich commented 3 years ago

I like this approach. I can see cases where you want both options: the most common case is probably "click anywhere on the region", but if you have a bunch of overlapping regions, it might be convenient to switch to 'click on center' mode so that you can select a region that's behind another (as long as they're not concentric). This is an issue I run into pretty often w/ds9 region editing; the only way to handle it there is to explicitly move a region to the front/back - which is fine, but it would be neat if there were another approach.

On that note - do you know how overlapping regions work with selectors?

dhomeier commented 3 years ago

Yes, I admittedly haven't felt a strong need for that behaviour, but I also can hardly imagine a situation were dragging from anywhere inside the region would be confusing or counterintuitive. And when combining it with rotation, it's definitely more awkward having to press space and r simultaneously. Perhaps there is demand for a complementary functionality (on the matplotlib side) to also temporarily deactivate this in drag_from_anywhere=True selectors with space or another key, but I am not sure how straightforward this would be to implement.

I haven't tried anything with overlapping (or any multiple) regions so far, sorry.

dhomeier commented 3 years ago

I haven't tried anything with overlapping (or any multiple) regions so far, sorry.

Well, at least interactively this seems to become tricky, as all regions (overlapping or not) will try to interpret the mouse interactions simultaneously! May need further work to connect the mouse/key event to one selector at a time.