select_points / session-ful marking

[eteq]

This is forked from https://github.com/eteq/astrowidgets/pull/1#issuecomment-406811166 + related discussion in #10.

Specifically, about the select_points function from the API - it was replaced by w. is_marking = True.

My argument is that, while having marking be settable is tempting at first glance, I think it's better as a method because it's actually doing something. That is, the typical user (I think) isn't thinking about the act of marking stuff as a state machine, but rather a set of actions. Most workflows are either:

workflow 1:

1. show an image
2. mark some points/regions on it that are defined in a table or similar that requires no interaction
3. zoom around/look at what's marked
4. possibly repeat 2-3, or workflow 2

workflow 2:

1. show an image
2. interactively click on some objects in the image
3. do something in code with the marked locations
4. possibly repeat 2-3 for a different set of points, or workflow 1

To be very concrete one might select a subset of stars that are good PSF stars, and then use them to build a PSF (workflow 2), then the user wants to mark all the new locations of the stars after re-centroiding/fitting (workflow 1), then they want to mark a different set of stars from that set that turn out to look bad from the fitting (workflow 2 again).

So that sort of use case motivates select_points/mark_points. It's not modal per se (at least not to the user), but rather they start the action of selecting points, then they finish and are done. Then the next time they want a different set of points.

TL;DR: While I am fine with the name changes, I think select_points/mark_points is still useful, and it should retain the behavior of knowing which "session" the selections were made from. Perhaps get_markers returns all of them, but get_last_markers() or something like that returns only the most recent set? Or get_marker_sessions() returns a list of lists of markers?

[pllim]

If we want to support "sessions", then we also need to support merging/splitting/deleting/saving/loading sessions? Or am I over-thinking this?

[mwcraig]

I'm also inclined at this point to switch from making self.is_marking a settable property to adding a method called start_marking for a few reasons:

• I'm encountering cases where it would be handy to:
  • name different sets of markers that are added interactively. (.e.g by labeling them "bad comparison star" or something).
  • set the marker properties along with the name
• Better symmetry with stop_marking, currently part of the API. Right now one could do either .is_marking = False or .stop_marking().

The first point requires either switching to a method to initiate marking or creating some kind of Marking object for specifying marker metadata.

The second point could be addressed by removing stop_marking but that method has an option to clear the markers when marking is stopped.

@pllim -- if you give this change a thumbs up I'll implement it this morning.

[pllim]

@mwcraig , since you have a real use case for this, 👍 from me. Thanks!