search

ejeschke / ginga

The Ginga astronomical FITS file viewer
BSD 3-Clause "New" or "Revised" License
120 stars 77 forks source link

Incorporate astropy.visualization in Ginga's image scaling #560

Open pllim opened 6 years ago

pllim commented 6 years ago

Partly address #555

ejeschke commented 6 years ago

I'm not sure that I am ready to do that. I'd prefer that the core widget does not depend on astropy. It's a general toolkit for scientific visualization and there is no particular need for that part to depend on astropy. I'm totally fine with ginga using astropy for astronomy plugins or other parts of ginga that will be used for astronomy.

I believe the glue viewer does not even use the astropy-helpers toolkit, and it is considered an astropy affiliated project.

pllim commented 6 years ago

Re: Glue -- Hmm, good point considering its integration is "green" over at glue-viz/glue#1407 . @astrofrog and @eteq , could you please clarify?

pllim commented 6 years ago

FWIW astropy.visualization comes "free" as astropy is a required dependency, so it is always there anyway.

astrofrog commented 6 years ago

Using astropy-helpers is not a requirement for affiliated packages. Glue does use astropy.visualization for the image viewer: https://github.com/glue-viz/glue/blob/master/glue/viewers/image/composite_array.py#L9

pllim commented 2 years ago

If we want to revisit this, needs some sort of translation layer to https://github.com/ejeschke/ginga/blob/master/ginga/ColorDist.py , maybe.

Also see https://docs.astropy.org/en/stable/visualization/normalization.html

pllim commented 2 years ago

Maybe https://github.com/ejeschke/ginga/blob/master/ginga/AutoCuts.py too.

eteq commented 2 years ago

I'm also curious about

I'd prefer that the core widget does not depend on astropy

in the context of an optional dependency, would this be OK? I.e., if the astropy machinery is used if its present and the user passes in this machinery.

ejeschke commented 2 years ago

in the context of an optional dependency, would this be OK? I.e., if the astropy machinery is used if its present and the user passes in this machinery.

That is fine.

I think it would be straightforward to make a ColorDist or AutoCuts class that uses the astropy visualization stuff. These are just objects that are called from the viewer, so they are like black boxes in a sense--you could replace them with a different instance as long as the class presents the same API.

pllim commented 2 years ago

In principle, both Ginga and Astropy methods takes the input data and spits out modified data, so plug and play might be possible. But it is not obvious to me where this should happen. I tried to code dive a little and got lost in all the callback handling. Modifying ginga/ColorDist.py might be a level too deep. I am thinking more like a viewer-level operation, where the viewer would change the call depending on whether the algorithm set is a string, Ginga object, or Astropy object. And given this only affects widgets with programmatic user controls, we don't even need to worry about backends like Qt or GTK. @ejeschke , any ideas where to start?

pllim commented 2 years ago

I started astropy/astrowidgets#148 to try to understand how this feature will be used downstream. @mwcraig and @eteq , feel free to correct me if I misunderstood. Thanks!