adding ccdproc

[crawfordsm]

This is the first draft for the CCDPROC repo. I know that @astrofrog @eteq @adrn @perrygreenfield @keflavich are interested in this and it is directly related to astropy/astropy#448, but let me know if I missed anyone.

[crawfordsm]

Three big outstanding questions I have:

1. The CCDData object will definitely inherit from NDdata and will include the uncertainly methods from that. So this is related and needs to look at the pull request associated with that. However, what are people's feelings about what uncertainy to use (variance, inverse variance, etc)?
2. Most of the processes aren't interdependent but just short hand helpers. My very initial version was doing everything as functions but then I switched to a more class format for some of them. I'm not really tied to either case but it would be good to hear about people preferences.
3. How should transformations be handled?
   -What are people's preferred methods at the moment?
   -Should it only be handled by the WCS? (ie never actually touch the data)

[keflavich]

1 - I would vote for variance since it should usually involve the fewest additional operations when propagating error (right?).
3 - scipy.map_coordinates ? There needs to be a component of transformations that projects to a new pixel space.

[perrygreenfield]

1. Most of the processes aren't interdependent but just short hand helpers. My very initial version was doing
   everything as functions but then I switched to a more class format for some of them. I'm not really tied to either
   case but it would be good to hear about people preferences.

I'd argue the class should hold attributes for the ccd, but the steps should be functions that expect that object to operate on. I think it is good to decouple the implementation of the steps from the class. It makes it easier for people to write their own custom steps without having to worry about dealing with how to handle the existing methods. Then the sequence of steps to use in a given reduction pipeline can be also defined in different ways. I see these 3 things as fairly independent (CCD, processing steps, and a particular sequence of steps for a given reduction).

1. How should transformations be handled?

Should it only be handled by the WCS? (ie never actually touch the data)

WCS only. There are many ways people would like to resample images, and that dependence shouldn't be wired into the CCD class

[mwcraig]

Each of the steps outlined so far seem reasonably clear--one question I have is how metadata is used as part of the individual steps. I'd lean towards building in as much consistency-checking as possible on the assumption that other users will be as error prone as I am :). For example, dark subtraction could, in principle, check that the exposure times match, or scale as needed, and check that temperatures match.

In practice, I could imagine it being next to impossible to know which FITS keywords represent which quantity. Perhaps if there are a few key observational parameters we care about, a mapping could be done when the objet is created:

ccddata = fromFITS('img.fits', exposure='EXPOSURE', filter='FILTER', image_type='IMAGFETYP') 

which set properties for the CCD object that can also be set directly if desired:

ccddata.exposure = 30 * u.second
ccddata.ccd_temp = 233 * u.Kelvin
ccddata.image_type  = 'BIAS'          # though maybe better here to define a limited set of image types

[crawfordsm]

At the time I submitted it, I think my feeling was that I didn't want to limit the user by what they could do as data formats can vary from different instruments. However, I think you do make a good point and some more functionality could be added to certain functions, like the dark function, to prevent .

Since the information is in the headers and will be in ccd.meta, I would probably imagine handling it as following:

ccddata=subtract_dark(ccddata,darkframe, exposuretime_keyword='EXPTIME', scale=True )

Such that the user has the option of passing the keyword and scaling to subtract dark.

[eteq]

This may be a dated question, but the answer is: astropy.units.Quantity - it's either a scalar + unit or an array + unit, depending on what you feed it at the time of creation. You definitely want to use those wherever it makes sense, as that's one of the ways a lot of astropy is "glued together"

[crawfordsm]

In a first bit of code that I was writing on this, that is what I did although I did have to create some of the units like adu.

[eteq]

Ah, ok, cool. There is an adu unit in astropy.units.astrophys, though (http://docs.astropy.org/en/stable/units/index.html#module-astropy.units.astrophys) - or did you add that for this purpose?

[crawfordsm]

Well, that's one less line of code that is needed now. Thanks for pointing that out!

[eteq]

And on the question from @mwcraig about how you might get at the EXPTIME: I'm not sure I like the idea of actually requiring it to be in meta and hence having to specify a keyword. I would like either of these be possible in-code:

ccddata=subtract_dark(ccddata,darkframe, exposuretime=2*u.minute, scale=True)
ccddata=subtract_dark(ccddata,darkframe, exposuretime='EXPTIME', scale=True)

The problem is knowing when exposuretime is a keyword to look up in meta, and when it is supposed to actually be a string. A secondary problem is knowing what units 'EXPTIME' from the FITS file is in (for 'EXPTIME' there's a well-known quasi-standard, but that's not always true). So maybe add in a new object that you could use something like this:

ccddata=subtract_dark(ccddata,darkframe, exposuretime=ccdproc.Keyword('EXPTIME', u.sec), scale=True)

Internally, Keyword would be a simple class that would be used like this inside subtract_dark:

if isinstance(exposuretime, Keyword):
    exposuretime = exposuretime(ccddata.meta)

and it would output a Quantity with seconds units (or just pass the value from meta through if you do Keyword('SOMEKEY'))

[crawfordsm]

I definitely like the idea of a Keyword class so it isn't just a string that is being passed and that it provides an easy way to check what type it is. I think that would be a great way to balance the ideas of different users

[mwcraig]

I'll write the next draft of the API to include this class--I already have a Keyword class written for another package that would need minimal modification to work here (and includes the concept of a keyword synonym, essentially a list of names the desired keyword might have).

[eteq]

Sounds great, @mwcraig !

[embray]

Could this be called from_fits instead?

@astrofrog Might want to chime in here it but might also be better if it registered a FITS reader via the I/O registry; though it might not hurt to have an explicit from_fits method as well, which the I/O handler would simply wrap.

[astrofrog]

@embray - I agree, if CCDData is an NDData sub-class, it should define it's own reader/writer and then use CCDData.read

[crawfordsm]

@astrofrog can you point me to an example of this so I know what needs to be added?

[astrofrog]

@crawfordsm - yes, this is described here: http://docs.astropy.org/en/stable/io/registry.html

We're planning on refactoring how this is done a little, but it won't change too much.

[embray]

I still don't see the point of having a specialized class for this. Maybe I'm missing something but it seems like the only reason for its existence is a workaround for the fact that astropy.io.fits doesn't directly support units (yet).