astropy / astropy/regions

Outstanding work on interactive regions functionality

Open
#391 3 comments 1 reaction 0 assignees View on GitHub
visual_editing
Dominant language
Python
Stars
65
Forks
64
Avg merge
2d 13h
Merged PRs (30d)
4

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:

- [x] Use the region's visual properties for the selector
- [x] Changelog entry
- [x] Tests
- [x] Expand approach to other region types
- [ ] Investigate how to deal with Sky regions and WCSAxes
- [x] 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 https://github.com/matplotlib/matplotlib/pull/19657 has added an option to always enable this behaviour; currently this can be activated by
```python
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.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with region.as_mpl_selector and the Matplotlib _SelectorWidget and RectangleSelector documentation cited in the issue. Review the remaining Sky regions and WCSAxes work, along with the kwargs handling discussed in #390 and #392. Done means the agreed behavior is implemented and covered by the existing selector tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
matplotlib, python
Domain
data-visualization
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.