First go at spectral WCS

[wkerzendorf]

So I have used astropy.modeling to have a first go at making a spectral wcs which for now existed in a slightly broken form. It includes an interpolation method that takes into account the precise description of the WCS to interpolate from one to the other. It's still easy to use and most of the difficult handling is hidden from the user:

    >>> flux = np.random.random(1000)
    >>> wave = np.sort(np.random.random(1000))
    >>> spec1d = Spectrum1D.from_array(wave, flux)
    >>> spec1d = Spectrum1D.interpolate(Spectrum1DLinearWCS(6000, dispersion_delta=1, pixel_index=np.arange(1000)))
    >>> #Now it can be written to a fits file as it is linear encoded which is representable in FITS headers.

This is more of a preview version and the next step is to incorporate units tighter into this as well as making the wcs write to fits (and have a little function that reads from fits).

@keflavich and @nden - your thoughts.

[keflavich]

I'm having a bit of a hard time following these changes, in part because I don't think the class names are all preserved above the +/-'s in the diff. But from what I can tell, you've actually implemented a conversion between a lookup table and a simple linear model - which is what we had before - but now using wcs.invert instead of the more involved scipy approach? So far, looks good, but I am unclear where, if anywhere, astropy.models is actually being used. Is it just for the parameter handling, or is there some WCS fitting going on as well?

[wkerzendorf]

@keflavich: I'm borrowing the structure and the fitting capabilites of Models by subclassing from it. I don't know if they have a linear model so I made my own. For the Chebychev and the legendre case we can subclass from those models. I'm still building around it - it's incredibly flexible, but I'm not 100% happy with the API.
I think if you like the general idea, I would like to merge sooner rather than later and then continue to mold.
I want to make this more fully fledged before writing to astropy-dev and seeing what their response is.

It would also be interesting to see how the 2d WCS will work out.

[keflavich]

OK, I'm happy enough seeing this merged in - I'd rather see rapid development now to create some base capability with the understanding that we will probably need to majorly refactor later.

astropy.modeling.Poly1Dmodel is probably what you want though. astropy.modeling.Chebyshev1DModel and astropy.modeling.Legendre1DModel also exist.

[wkerzendorf]

I will merge once i have comments from @nden

[nden]

@wkerzendorf Sorry I did not respond earlier. I'll have a look today.

[wkerzendorf]

Hi @nden, no problem at all. I never expect people to reply on the weekend.

[nden]

@wkerzendorf @keflavich

I think this will work and is a good first prototype.

I was imagining something slightly different for the WCS though. I started writing a description of a WCS API which I hope to post next week. I got comments from a couple of people in my branch which I atill need to incorporate before posting it.

In any case, I am thinking of a WCS object as having as a minimum a transform and an output_coordinate_system. The transform is an instance of modeling.Model (and may be composite). So for the case you are writing I would not subclass from Model but have the model be an attribute of the WCS object. The reasoning behind this is that the auxiliary information (like coordinate system and units but other information too) is also important. It may be used, for example to transform to a different coordinate system for the purpose of comparing two observations. In addition the WCS has to handle multiple transforms e.g. for the case of IFUs (but not limited to that use case).

Specifically for the classes here, I am glad to see a Lookup table implementation. It's been high on my list of things to do but had no time for this so far. I think it is so often used that it deserves to be a separate model in modeling.
As it is currently written it can be easily transferred to a model (because your WCS object subclasses Model).
What do you think?

Having a lookup_table_interpolation_kind parameter is fine but I am wondering if we should not have another optional parameter which is an actual interpolator and leave its implementation to the user. In this way we provide a way to use different interpolation techniques and don't tie the implementation of the model to a specific interpolation. We could still provide a default 'linear' case.

I will send you the WCS API for comments next week if you are interested and have a chance to look at it.

[nden]

What I said above does not mean you should not merge and go on. I agree with @keflavich that it is important to have some basic implementation working. It will show what works, what not and what is needed. Refactoring is unavoidable.

[wkerzendorf]

@nden @keflavich: I'll merge now. One thing that I think we should think about next is a slight change to the model api: astropy/astropy-api#9