get_aca_images() returns MaskedColumns only for data with masked values (except IMG)

[taldcroft]

Description

Change get_aca_images() so that it converts the MaskedColumn columns from get_aca_packets to Column if there are no masked elements.

The only columns that can actually be missing are: 'BGDRMS', 'TEMPCCD', 'TEMPHOUS', 'TEMPPRIM', 'TEMPSEC', 'BGDSTAT'. These columns are not present in 4x4 images, so any query that includes 4x4 images will have these masked.

The 'IMG' column is still always masked because the mask has a slightly different meaning, namely providing the data mask for 4x4 and 6x6 images.

Unrelated fix

Functional testing with kalman_watch showed a problem with #181, namely that merging (vstack) of image tables with a times metadata caused warnings:

WARNING: Attribute `times` of type <class 'cxotime.cxotime.CxoTime'> cannot be added to FITS Header - skipping [astropy.io.fits.convenience]
WARNING: MergeConflictWarning: Cannot merge meta key 'times' types <class 'cxotime.cxotime.CxoTime'> and <class 'cxotime.cxotime.CxoTime'>, choosing times=<CxoTime object: scale='utc' format='date' value=['2024:308:01:44:40.211' '2024:308:02:19:13.786']> [astropy.utils.metadata.merge]

So this PR changes that times meta attribute to be opt-in for testing via an undocumented keyword arg set_times_metadata.

Interface impacts

Column types are changed.

• aca_view does not call get_aca_images().
• The only other production code calling get_aca_images() is kalman_watch, so this is functionally tested.

Requires

Requires masters environment (at least CxoTime.linspace()).

Testing

Unit tests

• Mac (new tests)

(masters) ➜  chandra_aca git:(mask-only-masked-data) git rev-parse --short HEAD                                 
d65fc41
(masters) ➜  chandra_aca git:(mask-only-masked-data) pytest
============================================== test session starts ===============================================
platform darwin -- Python 3.11.8, pytest-8.0.2, pluggy-1.4.0
rootdir: /Users/aldcroft/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 221 items                                                                                              

chandra_aca/tests/test_aca_image.py .................                                                      [  7%]
chandra_aca/tests/test_all.py ........................                                                     [ 18%]
chandra_aca/tests/test_attitude.py .............................................................           [ 46%]
chandra_aca/tests/test_dark_model.py ............                                                          [ 51%]
chandra_aca/tests/test_drift.py ..........................                                                 [ 63%]
chandra_aca/tests/test_maude_decom.py .........................                                            [ 74%]
chandra_aca/tests/test_planets.py ...............                                                          [ 81%]
chandra_aca/tests/test_psf.py ...                                                                          [ 82%]
chandra_aca/tests/test_residuals.py ss...                                                                  [ 85%]
chandra_aca/tests/test_star_probs.py .................................                                     [100%]

======================================== 219 passed, 2 skipped in 45.03s =========================================

Independent check of unit tests by Jean

• Linux

(ska3-masters) jeanconn-fido> pytest
============================= test session starts ==============================
platform linux -- Python 3.11.8, pytest-7.4.4, pluggy-1.4.0
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: timeout-2.2.0, anyio-4.3.0
collected 221 items                                                            

chandra_aca/tests/test_aca_image.py .................                    [  7%]
chandra_aca/tests/test_all.py ........................                   [ 18%]
chandra_aca/tests/test_attitude.py ..................................... [ 35%]
........................                                                 [ 46%]
chandra_aca/tests/test_dark_model.py ............                        [ 51%]
chandra_aca/tests/test_drift.py ..........................               [ 63%]
chandra_aca/tests/test_maude_decom.py .........................          [ 74%]
chandra_aca/tests/test_planets.py ...............                        [ 81%]
chandra_aca/tests/test_psf.py ...                                        [ 82%]
chandra_aca/tests/test_residuals.py .....                                [ 85%]
chandra_aca/tests/test_star_probs.py .................................   [100%]

======================== 221 passed in 95.37s (0:01:35) ========================
(ska3-masters) jeanconn-fido> git rev-parse HEAD
d372a2766f39565a639dbe079e91f7f2016755d6

Functional tests

Ran kalman_watch task_schedule tasks on Mac in masters environment with this PR in the PYTHONPATH.
No errors or warnings were produced and the outputs look reasonable:
https://icxc.cfa.harvard.edu/aspect/test_review_outputs/chandra_aca/pr-182/kw3/

[jeanconn]

Good catch on the kalman_watch warning introduced by #181. Should we add a vstack test?

[taldcroft]

Good catch on the kalman_watch warning introduced by #181. Should we add a vstack test?

I already spent way too much time on this. The issue is really a bigger one that we have to be generally careful about adding metadata to Tables that might be used in table operations. But I'm pretty sure we won't fall into that trap again in this particular function, so adding the test isn't likely to help.

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 suggestion.

Comments skipped due to low confidence (1)

chandra_aca/maude_decom.py:1408

• [nitpick] The 'set_times_metadata' parameter is not mentioned in the docstring of 'get_aca_images'. Consider adding it for clarity, even if it's undocumented.

set_times_metadata = kwargs.pop("set_times_metadata", false)