Add Save Method to NDCube

[DanRyanIrish]

Add a save method to NDCube with the following API:
NDCube.save(self, filename, file_type="FITS"):
For now it should just support FITS. More file types can be added later. The method should save the information in an NDCube, including data, mask, uncertainty, unit, extra coordinates, WCS, etc., in a standardized way.

[Cadair]

we will have inherited the write method from NDData, I would be interetsed to see what it can do.

[DanRyanIrish]

Good point, I completely forgot about that! But it seems NDCube has not inherited that. It seems the write method is part of NDDataRef, not NDData. See http://docs.astropy.org/en/stable/nddata/
Anyway, NDDataRef.write seems to be for writing to FITS files specifically. It could nonetheless be used in NDCube and hence irispy.

[yashrsharma44]

Hey @DanRyanIrish ,can NDCube subclass NDIOMixin to support the I/O of fits file?
I would love to make a pull request for this.

[DanRyanIrish]

Hi @yashrsharma44. I'm really glad to hear you are interested in addressing this issue and would love to see you write a PR. I'd be very interested to hear more about what NDIOMixin provides. As I understand it just registers read and write methods with the astropy.io.registry rather than actually provides the instructions on how to read or write. Is this correct? If so, can you explain the advantage of inheriting NDIOMixin rather an just writing a write method within ndcube itself?

[yashrsharma44]

Sorry my bad ;), you were right regarding the interface of NDIOMixin. After a reading through the docs of astropy for NDIOMixin , it was clear that it simply provides the registry of read and write methods.
So subclassing NDIOMixin does not provide the implementation( as I wished ;) ).
However, we can still subclass NDIOMixin as it creates the registry of the read and write methods.
The major task remains, is to code up the write function for the above purpose.

[DanRyanIrish]

OK. Well it's good to have that confirmed. Would you still be interested in working on a PR to do this?

[yashrsharma44]

Yeah ! I would need some guidance regarding writing up the read write method.

[DanRyanIrish]

OK, great. In that case I can write up an outline of how the write method should work and then we can discuss.

[yashrsharma44]

Hey @DanRyanIrish , can we discuss the above in some common mailing list?

[DanRyanIrish]

Hi @yashrsharma44. I think it's best to discuss it here so anyone who's interested can see. Then we can update the description at the top of the issue with decisions for easy reference.

[DanRyanIrish]

So an NDCube is comprised of data, uncertainty, mask, meta, wcs, unit, missing_axis, and extra_coords. Let's ignore the extra_coords to start with as they may pose some difficulty.

One structure for saving and NDCube into a FITS file would be:

• hdu0:
  • header: meta, wcs
  • data: None
• hdu1:
  • header: unit
  • data: data
• hdu2:
  • header: unit/uncertainty_type
  • data: uncertainty
• hdu3:
  • header: None
  • data: mask

This I think is pretty intuitive for FITS files. We haven't said where missing_axis or extra_coords can be stored. Perhaps missing_axis can be stored in the hdu0 header with the WCS info in the following way:
MISSING0: 1
MISSING1: 0
...
MISSINGN: 0
where the values are 0, 1 indicating False or True.

As for the extra_coords, there is no limit to the number of them, and they can apply to up to N dimensions. So perhaps all hdus after hdu3 be defined as separate extra_coords? E.g.

• hdu4
  • header: coord name, WCS axis/axes to which the coord corresponds, extra coord unit
  • data: extra_coord values
    ...
• hduN
  • header: as above
  • data: as above

Issues I see that have no been addressed in the above plan include:

• How to represent uncertainty and mask when their value is None. Do we simply have empty data or do we replicate 0-filled arrays? Probably the former to not make the fill unnecessarily large. Then when reading an NDCube FITS file, an empty uncertainty or mask hdu is interpretted as None when initializing an NDCube.
• Ensuring that header keys and values adhere to FITS standards. Or whether we want to enforce that...
• Possibly others I forgotten...

What do you think @yashrsharma44? I think we should also get @Cadair's feedback on this.

[yashrsharma44]

Sorry for missing out on the discussion!
Well, I think that the proposed structure for storing NDCube data is reasonably well designed, so I don't expect any flaws in that.
Here are some pointers that I would like to share -

• We should not leave the hdu2 for the uncertainty blank, rather use some dummy value for defining None value. This would help in maintaining the structure of fits generated from NDCube. Having the header for the uncertainty should also work, with the data attribute as blank.
• Same as above for hdu3 for storing mask
• Storing missing_axisin the hdu0 would be good, as knowing beforehand if there would be some missing axis due to dependent wcs axis, would be fine.
• We can enforce FITS standard using the is_fits() function from astropy.io.fits.connect
• Defining N number of hdu should be intuitive for storing extra_coords

Some queries from my side -

hdu0:
header: meta, wcs
data: None

What are the different key/value pairs would we store for wcs, besides including some default key/value pairs, and the meta attributes?

WCS axis/axes to which the coord corresponds

Can you elaborate on this ?

Ping @DanRyanIrish
If the final writeup is fairly good, then can I start with working on the PR? Though I think, I should wait for @Cadair 's feedback.

[DanRyanIrish]

Hi @yashrsharma44. I'm not sure @Cadair needs to give feedback at this time. Let's push ahead with this plan!

[yashrsharma44]

Hey @DanRyanIrish , I forgot to ask, what type of FITS data structure are you planning to prefer to store the data for writing NDCube objects?

[DanRyanIrish]

The data, uncertainty, and mask can be primary/image HDUs. @Cadair suggested the extra_coords could be combined into a single Table HDU.

[DanRyanIrish]

@hayesla and @grahamkerr request this functionality in #409. Their conversation has been redirected here. Although note that much of the above discussion was had in the context of ndcube 1.x. In #409 @hayesla said:

After a recent discussion, I had with @grahamkerr, there may be a use case for write functionality of an NDCube (e.g. to a fits file). From what I can tell there doesn't seem to be one at the moment (however maybe I'm missing it?).

The easiest thing I guess would be to save the wcs and meta to a fits header and then save to a fits file with the nd-data array. However this may also not be a good idea, and maybe hard to generalize for sequence/collections.

But something to think about - any thoughts @DanRyanIrish @Cadair

@hayesla, as you see from this thread, we agree with you that a save/write method or function would be very useful. What you suggest would be the easiest thing to do but given the very generalised nature of NDCube there is more to consider. See the above discussions. Since those discussions were had, ndcube 2.0 has been largely implemented, which means we also need to think about how to save the ExtraCoords and GlobalCoords information. I'm sure all these issues are resolvable, but I think we shouldn't try until after ndcube 2.0 is released and the ExtraCoords and GlobalCoords implementations are finalised.

In the meantime, however, if you have more to add about how a write method could be implemented feel free to chime in here. If you would like to work on implementing it once 2.0 has been released, please let us know!

[Cadair]

It's worth remembering that you would only be able to save a NDCube backed by an astropy.wcs.WCS to a FITS file without jumping through a lot of hoops a gWCS is not serialisable to FITS.

[DanRyanIrish]

That's a very good point. So in APE-14 land there is no generalized way to save to FITS? Given the scope of ndcube, that makes me unsure about a save to FITS method. Although perhaps how useful it could be to users might still be a reason to provide it.

[DanRyanIrish]

What about HDF5? If gWCS can be serialized to that and we provide a reader as well, then maybe we don't need to provide a FITS writer.

[Cadair]

gWCS will serialise to asdf, but currently there is no way to save a FITS WCS to asdf (although that could be developed). As I am aware asdf is really the only serialisation option for gWCS at this point.

So in APE-14 land there is no generalized way to save to FITS?

There is no serialisation at all in APE 14, it's very likely that all underlying implementations of APE 14 would have their own serialisation formats.