translator for specreduce Trace objects

[kecnry]

This PR implements a new translator for specreduce Trace objects. In doing so, it adds specreduce as a dependency. In order to round-trip and retrieve the same underyling Trace object (instead of always having to export to an ArrayTrace), this currently saves a copy of the input Trace object in the meta of the glue Data object. If this is not preferred, we could create an ArrayTrace from the glue Data object, but we'll likely still need to attach an instance or reference to the input data.

[dhomeier]

Do you think you could set up a test for this with a basic example Data set?

[dhomeier]

Thanks for this contribution, this looks all quite good to me, although I am not familiar enough with the Trace format to say if this is the best possible implementation. It certainly deserves a changelog entry as well, perhaps under 0.5.0 (unreleased) for a new feature.

The main points I see remaining to be settled are

1. Adding specreduce to requirements; with specutils and spectral-cube already runtime dependencies and specreduce now an Astropy-coordinated package as well, I don't see much problems with that.
2. As you mentioned, adding the full trace to the meta data might be controversial. As I see it at the moment you are storing a full copy in data.meta['Trace'], while data.meta['Trace'].trace is at the same time kept in data['y'] (this might additionally be checked in the tests BTW). Wondering if it would be preferable to strip the trace itself from the meta copy and add it back in later in to_object to save some space, but I have no idea if it's worth the effort at all for typical sizes of these objects – I guess they are usually (n-1)-D, so not that large? Possibly there is not even a real memory impact if this is a shallow copy.

Leaving those open for discussion with @astrofrog.

[eteq]

(Oh, and my "If there's some way that glue data objects can flag "have been modified since created", only do the next step if that one is true." comment could use some insight from @astrofrog if @dhomeier doesn't know the answer!)

[codecov[bot]]

Codecov Report

Merging #72 (aec7ee3) into main (bae4ecf) will decrease coverage by 0.16%. The diff coverage is 92.68%.

:exclamation: Current head aec7ee3 differs from pull request most recent head 7ce7142. Consider uploading reports for the commit 7ce7142 to get more accurate results

@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
- Coverage   97.63%   97.46%   -0.17%     
==========================================
  Files          15       17       +2     
  Lines        1182     1223      +41     
==========================================
+ Hits         1154     1192      +38     
- Misses         28       31       +3     

Impacted Files	Coverage Δ
glue_astronomy/translators/trace.py	85.71% <85.71%> (ø)
glue_astronomy/translators/__init__.py	100.00% <100.00%> (ø)
glue_astronomy/translators/tests/test_trace.py	100.00% <100.00%> (ø)

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

[dhomeier]

Oh, one last thing: please add a changelog entry, and change the current section heading to 0.5.0 (unreleased)! :)

[dhomeier]

Successful test run, so considering the last commit a [ci skip].

[eteq]

sorry I didn't get back to this sooner, by a a post-hoc "yes this is good, thanks!"