search

astropy / astrowidgets

*PRE-ALPHA*/heavy dev. Jupyter widgets leveraging the Astropy ecosystem
https://astrowidgets.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
31 stars 14 forks source link

Setting stretch as a string is problematic if the stretch requires a parameter #146

Open mwcraig opened 3 years ago

mwcraig commented 3 years ago

While setting the stretch using a string (e.g. viewer.stretch = 'sqrt') is very convenient, saome stretches require parameters (e.g. PowerStretch) and many have optional parameters (e.g. AsinhStretch).

I don't see a way to handle that without one of these:

  1. add some more public properties that correspond to the settings for these stretches
  2. make a method set_stretch instead that allows optional parameters, and a get_stretch
  3. Allow stretch to be set to either a string or an astropy stretch object.

Of these, 1 seems confusing, 2 is straightforward, 3 is maybe more convenient in many cases.

mwcraig commented 3 years ago

Actually, the original implementation called for an Astropy stretch to be the value, I think it was turned into a string to get ginga working quickly, maybe?

mwcraig commented 3 years ago

After looking this over some more I think it makes sense to go with 3....or maybe even disallow strings.

mwcraig commented 3 years ago

I see in #126 that there is no restriction imposed on what stretch can be. It might be nice to be explicit about what is allowed, though.

pllim commented 3 years ago

2 and 3 are basically the same solution, but just a matter of preferring method over attribute setter, or vice versa.

👎 on disallowing string for Ginga though. I don't see the point to have to manually build an astropy.visualization object when I can just pass in a string to it.

mwcraig commented 3 years ago

👎 on disallowing string for Ginga though. I don't see the point to have to manually build an astropy.visualization object when I can just pass in a string to it.

Agree, even for things other than Ginga. Almost all of the astropy stretches have defaults for optional parameters, so something like https://github.com/astropy/astrowidgets/pull/142/files#diff-6aea2ce078dbeb19a51bdb4018c2b963ffa8728ce163c5d91aeb3bf42a3a2293R337 works as long as users can change properties after the fact.