Adding support for uncertainty extraction to NDDataArray

[bmorris3]

In #81, I added translators for objects in astropy.nddata. In this PR, I add support for extracting uncertainties from glue Data objects and translating them into the uncertainty attr in NDDataArray objects. This was missing in the previous PR, but should have been there.

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.34 🎉

Comparison is base (63d9e0c) 96.99% compared to head (5469ccb) 97.33%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #86      +/-   ##
==========================================
+ Coverage   96.99%   97.33%   +0.34%     
==========================================
  Files          17       18       +1     
  Lines        1296     1351      +55     
==========================================
+ Hits         1257     1315      +58     
+ Misses         39       36       -3     

Impacted Files	Coverage Δ
glue_astronomy/translators/nddata.py	95.37% <100.00%> (+2.81%)	⬆️
glue_astronomy/translators/tests/test_nddata.py	98.56% <100.00%> (+0.14%)	⬆️
glue_astronomy/tests/test_spectral_coordinates.py	100.00% <0.00%> (ø)
glue_astronomy/spectral_coordinates.py	100.00% <0.00%> (+6.66%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[dhomeier]

Looks good for StdDevUncertainty. I admit I don't know exactly to what extent glue Data objects support other uncertainty types, but Specutils1DHandler processes and stores the types and units to retrieve the appropriate type:

glue-astronomy/glue_astronomy/translators/spectrum1d.py

Lines 275 to 277 in 71ded78

	if attribute_label == 'uncertainty':
	values = UNCERT_REF[
	data.meta.get('uncertainty_type', 'std')](values)

It would be good to cover this consistently, and perhaps add test cases for VarianceUncertainty and InverseVariance.
Then at some point we should probably see how the Spectrum1D translator can reuse the NDData functionality, but that need not be part of this PR.

[bmorris3]

@dhomeier I agree this would be great! For now, the relevant collapse functions I wrote for jdaviz via astropy in NDDataArray only support variance-like uncertainties, so this PR is a good first step (astropy/astropy#14175).

[dhomeier]

I see. Would it make sense to have to_data convert other types with represent_as(StdDevUncertainty) then? That's probably a bit double-sided, as other applications may have use for the original forms, even if jdaviz does not a this point.
But otherwise I think it should check for the correct type on input, as the present implementation would e.g. silently interpret an InverseVariance as StdDevUncertainty and return it as such.

[bmorris3]

Good idea @dhomeier! Implemented in 91d8dbc.

[dhomeier]

Great, thank! I'd still add the tests for the other input types.

[dhomeier]

Will need imports for the other uncertainties; also we may have to compare to spec.uncertainty instead of uncertainty since the former should have the correct units set (not sure if represent_as will transform it correctly otherwise).

[bmorris3]

@dhomeier Done and passing 💯

[bmorris3]

@dhomeier Would you consider this a bugfix, for soon release? We will use it downstream in spacetelescope/jdaviz#2039.

[astrofrog]

Looks good to me!