astropy / specreduce

Tools for the reduction of spectroscopic observations from Optical and NIR instruments
https://specreduce.readthedocs.io
61 stars 38 forks source link

Specreduce integration with jdaviz #109

Closed camipacifici closed 2 years ago

camipacifici commented 2 years ago

We have started discussing how to integrate specreduce and jdaviz. There are essentially two pathways:

  1. API first = magic function in the specreduce workflow that calls jdaviz at specific steps.
  2. Plugin first = specreduce in (probably) 3 plugins for tracing, background subtraction, and spectral extraction.

Each come with its pros and cons. In summary these are:

Here is the basic workflow in approach (1):

Open one (or many) 2D spectra.

Find trace

Subtract background

Re-find trace

Extract 1D spectrum

Here is the same thing for approach (2):

Open Specviz2D/Mosviz and load 2D spectra. Maybe 3 separate plugins: Tracing, Background subtraction, Spectral extraction. Interactions as above. Parameters can be specified manually in GUI and exported in notebook for reproducibility (like get_model_parameters). Ability to set same parameters for all the spectra in the Mosviz table.

Comments? @eteq @ojustino @pllim @kecnry @tepickering @duytnguyendtn and whoever else

pllim commented 2 years ago

I am all about testing and reproducibility, so my vote is API first.

That said, is it fair to lock in specreduce to a visualization package that is heavily under development? Should we make it opt in but allow people to still grab the data and, say, do it painfully in matplotlib if that is what they'd rather do?

camipacifici commented 2 years ago

I agree that people should still be able to use matplotlib at their own will.

tepickering commented 2 years ago

if we do want to implement a plotting API here (and i am not convinced we do), we need to keep it as general as possible. it will also have to support both interactive and non-interactive use. jdaviz is definitely not appropriate for the latter...

rosteen commented 2 years ago

That said, is it fair to lock in specreduce to a visualization package that is heavily under development? Should we make it opt in but allow people to still grab the data and, say, do it painfully in matplotlib if that is what they'd rather do?

My two cents is that specreduce should know nothing about jdaviz / not have jdaviz as a dependency, perhaps at most pointing to it in the docs as something one could use as a front end that exposes specreduce functionality. Basically I'm arguing that whatever we decide on (1) vs (2) from @camipacifici's suggestions, this issue/ticket should be a jdaviz issue, not in specreduce. Specreduce provides the backend functionality, which we then choose to expose in jdaviz.

rosteen commented 2 years ago

In regards to (1) vs (2), I think that means that I'm advocating for approach (2) to create a Jdaviz plugin that exposes the specreduce workflow through the GUI.

kecnry commented 2 years ago

I would agree that jdaviz-specific logic should live in jdaviz, and not here, although things we need in jdaviz may motivate changes or new features in specreduce. We can still implement the support in jdaviz with a user-friendly API first so that using specreduce within jdaviz can still emphasize reproducible workflows, etc.

pllim commented 2 years ago

Thanks for all the feedback. At 2022-06-28 tag-up, @camipacifici said this issue can be closed and discussions can happen in the Jdaviz JIRA tickets that have been created as a result of this discussion.

kecnry commented 2 years ago

See https://github.com/spacetelescope/jdaviz/pull/1514 for the first iteration of a spectral extraction plugin in jdaviz that makes use of specreduce.