astrowidgets

astrowidgets copied to clipboard

Right now there's a mix of a stateful and a non-stateful idiom in how you set the style of markers. self.markers is settable by the user, but it can also be set in start_marking. add_markers also uses self.markers, but does not have a markers keyword.

So I see two ways to fix this:

1. Add a markers keyword to add_markers
2. Do 1 and also make self.markers private
3. remove markers from start_marking

I really don't like 3 because I think it just makes sense to be able to set your marker style when you add the marks. So that leaves 1 and 2 - 2 is more "pure" (i.e., there's then one way to do it), but has the downside of breaking backwards compatibility (which we may or may not care about).

Triggered by spacetelescope/jdaviz#699

cc @pllim @mwcraig

I assume you mean marker not markers in most of this?

Yes, marker (no "s").

https://github.com/astropy/astrowidgets/blob/c594308246b5dde60db7900a532cf4cc53855756/astrowidgets/core.py#L482-L483

https://github.com/astropy/astrowidgets/blob/c594308246b5dde60db7900a532cf4cc53855756/astrowidgets/core.py#L438-L439

The only argument that I can see against removing the markers attribute is that it may sometimes be handy to use the same style markers for two different things, in which case would be helpful to be able to get the current marker in some way. That could be done by making it a read-only property or by adding a get_current_marker_style() method.

@mwcraig , how about something more explicit like get_marker_style(marker_name)?

I don't have strong opinions on this issue.