Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opportunities to simplify codebase by leveraging ndcube 2.3 #108

Open
DanRyanIrish opened this issue Jan 15, 2025 · 3 comments
Open

Opportunities to simplify codebase by leveraging ndcube 2.3 #108

DanRyanIrish opened this issue Jan 15, 2025 · 3 comments

Comments

@DanRyanIrish
Copy link

DanRyanIrish commented Jan 15, 2025

ndcube v2.3.0 is in the process of being released (a release candidate is already available). This includes some new features that could be very helpful to simplifying the eispac code-base in a few places and make it easier to maintain:

  1. Sliceable metadata: The new NDMeta object that can be attached to NDCube instead of a plain dict, allows metadata to be associated with an axis/axes, which can be one, all, or a subset of the cube's axes. The metadata can have a value for each pixel along the associated axis/axes, or simply be a scalar per axis. Moreover, if NDCube.meta is an NDMeta object, the metadata is automatically sliced by the NDCube slicing infrastructure. This could help simplify the metadata tracking in eispac.
  2. wavelength and radcal storage: A specific application of the sliceable metadata above, could be to move wavelength and/or radcal values into the metadata. Then EISCube.wavelength and EISCube.radcal could simply be pointers to relevant key in EISCube.meta. This means eispac wouldn't have to do the slicing of these attributes itself, as it would be done automatically in the meta by NDCube. However, perhaps there are other considerations which means it's better to keep one or both of these separate from the metadata?
  3. NDCube.rebin and EISCube.sum_spectra: As of ndcube v2.3.0, NDCube.rebin now accepts -1 as a shortcut to sum over an entire axis. This could make the implementation of EISCube.sum_spectra much simpler and easier to maintain. NDCube.rebin also enables summing (or averaging) over other axes, which could be used to sum over regions of the slit to increase signal-to-noise before performing spectral fits. If keeping the WCS as a FITS-WCS is a concern, see Role of "mod_index"? #109, which discusses how ndcube can help do this for eispac.
  4. apply/remove_radcal: NDCube now supports arithmetic operations with scalars, arrays and astropy Quantities. This could help simplify the implementation of EISCube.apply/remove_radcal by performing the multiplication/division and doing the unit handling. Updating the uncertainties and "notes" would still need to be done separately, so the methods would still be needed. But again, it may simplify the code-base, making it easier to maintain?

The release candidate for ndcube v2.3.0 can be installed via pip: pip install ndcube==2.3.0rc1 and a general release will be available via pip and conda in the near future.

Hope this helpful info, even if not for the ideas I've suggested above.

@DanRyanIrish
Copy link
Author

FYI ndcube 2.3 has now been officially released, in case anyone is interested for this issue or any other.

@MJWeberg
Copy link
Collaborator

Hello @DanRyanIrish. thanks for reaching out! These are some exciting and useful features! I will take a look and see how we can clean-up and simplify the code now.

On point (3), how does rebinning work with sliceable metadata? Will all grid-aligned metadata be rebinned too or will they just be sliced?

@DanRyanIrish
Copy link
Author

DanRyanIrish commented Jan 17, 2025

Hi @MJWeberg. That's a great question regarding point 3. To answer that, it's important to first note that there are two types of axis-aware metadata data: axis-aligned and grid-aligned. Axis-aligned is what we call it when there's a single value associated with each axis. So, an example is labels for each axis, e.g. ["slit step", "slit", "spectral"], for a 3D raster cube. Since rebinning does not change the number of axes, this type of metadata is unaffected.

The second type of metadata is grid-aligned. An example of this might be your corrected wavelength or radcal, which has a value at every pixel in the grid along a subset of axes. In this case, there is no general solution to how this metadata should be altered to represent the rebinning. So by default, NDCube.rebin just removes the axis information from the metadata but leaves the values unchanged. In this case, EISCube needs to implement its own rebin method that calls super().rebin and then handles the rebinning of specific grid-aligned metadata as needed.

In the specific case of the corrected wavelength, the "more philosophical correct" approach within the ndcube paradigm, would be to create a WCS that includes the corrected wavelength. However, if this is not smoothly varying, it might not be expressable as a FITS-WCS and so gWCS might be needed. If one of your requirements is that everything it writtable back to FITS files, this is not ideal. An alternative would be to extend ndcube.ExtraCoords to handle >1D tabular coords. But this is not a trivial effort. Therefore, "special-casing" the corrected wavelength as rebinnable metadata, might be the path of least resistance for you. This approach would still give you the advantage of ndcube's metadata slicing, without any extra coding effort from eispac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants