NDData objects

[crawfordsm]

Will imageutils tasks work on astropy.NDData objects (ie meaning that they will be able to handle bad pixel masks and uncertainty frames as well as just WCS) and not just ndarray objects?

I think this is a pretty critical aspect to make this package useful for other affliated packages. For example, we would have to re-wrap everything in here to work in the ccdproc affliated package.

Obviously, error handling can be difficult, but this aspect also would be what would make it truly useful for research.

[cdeil]

That's a tough question ... @keflavich, @larrybradley, @eteq and @astrofrog what do you think?

Personally I feel that it looks like the answer should be "yes, obviously imageutils should work for NDData". On the other hand it looks to me like NDData has been stuck in a not very useful state since it was created over a year ago ... I've never used it and don't see how using NDData would help with any of the analysis code I've written in Gammapy.

What would this mean in practice ... is there some boilerplate code we have to run on input / before output for every function that works with NDData? Or would each function be different and inspect the content of the NDData object before deciding what to do? Can someone point to a good example?

[keflavich]

@cdeil - I think imageutils should work on nddata objects. But, as you said, I have no idea what nddata obejcts are. Until they exist as objects we actually use - with a stable API - I don't think we can do anything about them.

[crawfordsm]

Background on NDData:
Documentation for NDData is available here: http://docs.astropy.org/en/stable/nddata/index.html

It was actually created during the original planning meeting for astropy and included in the first release. The whole motivation for setting it up was so for an object describing astronomical data sets that could be shared between different afflicated packages. There are some developments with it that have stalled out such as WCS (was waiting on the astropy.WCS package to become a bit more stable) and uncertainty handling (examples exist, but correlated errors are a real headache), but both are still useufl in their present forms. There is also some question about whether quantity should be used, but I believe those are more development issue which shouldn't effect the API at all. There has been recent updates and inclusions in it driven recently by the ccdproc development.

There are pro's and con's to using it, but for at least data analysis, I think it is absolutely critical to have bad pixel maps and error analysis. By having those built in at the ground level, it makes it much easier to propagate them through your code and simpler to build up pipelines. I find it is a lot easier to work on a CCDData object where the data quality has already been checked in creating the object, then including those checks in every bit of code. On the other hand, it does mean including error propagation into your code which can be difficult to do right.

Examples:
The ccdproc.ccddata class inherits from NDData object and the tasks in ccdproc were set up to all operate on a ccddata object, so if you want to see some examples of something using it, check out over there. For example:
https://github.com/astropy/ccdproc/blob/master/ccdproc/core.py#L45

I also think the sunpy developers have also made use of NDData as well. So there are at least a couple of examples.

[astrofrog]

I think the reason that NDData has stagnated somewhat is because it turns out that it's really hard to generalize some of the things we've been trying to do, such as arithmetic and uncertainty propagation. I feel as though it would be easier if the base NDData class was simpler than it currently is, and actually didn't include arithmetic and uncertainty propagation, then it would have a simple API and would be easy to support.

Another difficulty with NDData and the generalized WCS is going to be - what if imageutils needs to change the WCS? How do we tweak an an arbitrary WCS? It's easy to say that the routines here should accept NDData, but at the same time I think it's going to be very challenging to deal with the WCS aspect properly.

Anyway, I'm +1 on trying to accept NDData, but I think that we still haven't properly 'solved' NDData itself. But now is the right time to be thinking about this! Maybe we can restrict imageutils to a subset of NDData, for instance a 2-d NDData sub-class that only takes astropy.wcs.WCS instances as the WCS object, or things like that. Trying to deal with the fully general NDData is going to be a nightmare.

[astrofrog]

In terms of sub-classes, we could also restrict ourselves to n-dimensional NDData objects that have astropy.wcs.WCS (the current FITS-WCS) instances as the .wcs property. When the new WCS comes along, we can then re-assess how easy it is to support that too.

TL;DR: I think we should not support NDData, but a more specific sub-class so that we can make assumptions about the WCS.

[cdeil]

At the top of the NDData docs is a note:

The NDData class is still under development, and support for WCS and units is not yet implemented.

As far as I can see there's not even an issue on Github that someone plans to do this for Astropy 1.0.

Until this changes I'd suggest we keep using ndarray and wcs objects directly in imageutils ... it should be easy to wrap any functionality we have to work with NDData or a sub-class once it contains WCS.

As a little anecdote, I've considered using NDData once when I tried to write a Maps class for gamma-ray astronomy, but then decided to subclass HDUList (which was also a mistake and this class is currently broken):
https://github.com/gammapy/gammapy/blob/master/gammapy/background/maps.py#L20

The reason I didn't use NDData is because it doesn't support the data model for this application (multiple "data" images, Poisson statistics for the n_on and n_off images) ... I second @astrofrog's comments above that NDData shouldn't do arithmetic and error propagation .. the only reason it works well for CCDData is because it was written with the CCD data application in mind.

[cdeil]

If someone thinks using NDData inputs is feasible / helpful for image processing functions, you could try and use @astrofrog's reproject function as an example ... or @larrybradley's downsample and upsample functions that were merged into imageutils today (but don't show up in the online docs yet because of issue #14).

[larrybradley]

I think it would nice if imageutils functions could accept NDData as input, but it should not be required, especially since NDData is still incomplete/under development. The functions should also work with ndarrays, i.e. image inputs can be either ndarray or NDData.

[crawfordsm]

For example, the ccdproc.rebin similar to upsample works on both ndarrays and ccddata objects.
https://github.com/astropy/ccdproc/blob/master/ccdproc/core.py#L701

[astrofrog]

I have a suggestion which could work for now - how about 'loosely' supporting NDData via duck-typing, i.e. check if the data object passed has data and wcs attributes (and maybe mask) and if so, using those? Then it won't strictly require subclasses of NDData, but NDData-like objects.

In fact, maybe NDData should be an interface specification, not a base class, detailing what attributes should be available?

[astrofrog]

@crawfordsm - I don't understand the line rebin(nccd.uncertainty.array, newshape) in your example - surely this will only work if the uncertainty is a variance, not a standard deviation?

[crawfordsm]

Well, @astrofrog, this raises a point that yes, correct error propogation is hard. For example, the errors on those pixels is also correlated, and in reality, we need a way to have correlated error arrays. However, we have tried to take a stance of adding notes where the error propogation may not be correct (perhaps it should be warnings), but that could certainly be updated in the future for better error handling. The error handling is handling more correctly in some of the other tasks but I was just giving an example of something that works on both ndarray and ccddata. However, please do open an issue about it over at ccdproc.

[larrybradley]

@astrofrog I think they would need attributes for both a mask and an error image. I like the idea, but one question is what to call these attributes. In my functions, I've been explicitly clear and called them mask_image and error_image. NDData calls these mask and uncertainty. Are those stable names? (fwiw, I'm not sure I like uncertainty). I recall some discussion last year about using standard deviation or variance arrays.

[perrygreenfield]

My view, at least for now, is that NDData is useful as a standard container, but we shouldn't load it too much implied functionality. E.g., no arithmetic, no automatic error propagation, no automatic data quality propagation, etc. Slicing with an updated WCS would be a nice thing (and should be generally possible with gwcs I believe). There are just too many special cases to handle errors correctly. But providing error and data quality information is useful for those tasks that know what to do with it. How they use it certainly will depend on context.