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

Add color1 to agasc fetch in centroid_dashboard #306

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Oct 6, 2024

Description

Add color1 to agasc fetch in centroid_dashboard.

This is required because of the changes in https://github.com/sot/chandra_aca/pull/170/files .

As COLOR1 is now used the the proseco acquisition mask, if calling code is using just a subset of columns, it needs that column too. It is a different question about if the calling code should just be getting everything, but this fix to just include COLOR1 is trivial.

Fixes issue that the calls in centroid dashboard plotting will fail on the bad acquisition mask and end up with "KeyError: 'COLOR1' " if run in an environment with the modern chandra_aca and proseco with mask using COLOR1.

Interface impacts

Testing

Unit tests

  • Linux
jeanconn-fido> git rev-parse HEAD
44a8278a372c263f5f1286fd3c561266455214c9
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 108 items                                                                                                                   

mica/archive/tests/test_aca_dark_cal.py ..................                                                                      [ 16%]
mica/archive/tests/test_aca_hdr3.py .                                                                                           [ 17%]
mica/archive/tests/test_aca_l0.py ...                                                                                           [ 20%]
mica/archive/tests/test_asp_l1.py .......                                                                                       [ 26%]
mica/archive/tests/test_cda.py ..............................................                                                   [ 69%]
mica/archive/tests/test_obspar.py .                                                                                             [ 70%]
mica/report/tests/test_write_report.py .                                                                                        [ 71%]
mica/starcheck/tests/test_catalog_fetches.py ...............                                                                    [ 85%]
mica/stats/tests/test_acq_stats.py ...                                                                                          [ 87%]
mica/stats/tests/test_guide_stats.py ....                                                                                       [ 91%]
mica/vv/tests/test_vv.py .........                                                                                              [100%]

=================================================== 108 passed in 512.06s (0:08:32

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

I set the code around line 929 in centroid_dashboard.py to raise a generic exception instead of handling it with log and continue and then went through the motions to get data and plot for one obsid, showing the key error in the unpatched code and the lack of error in the patched code. Eventually centroid dashboard should get better unit tests.

In [1]: from mica import centroid_dashboard  
In [3]: metrics_obsid, metrics_slot = centroid_dashboard.get_observed_metrics(25516, metrics_file='test_file.dat') 
In [5]: kwargs = {"factor": 20, "save": True} 
In [8]: import os                                                                                                                      
In [9]: os.makedirs('obs_test')   
In [10]: centroid_dashboard.plot_observed_metrics(25516, plot_dir='obs_test', coord='dr', att_errors=metrics_obsid["att_errors"], **kwargs) 
...
    265 elif isinstance(item, (int, np.integer)):                                                                                          266     return list(self.values())[item]                                                                                                                                                                                                                                  KeyError: 'COLOR1'

Patched

In [1]: import os

In [2]: from mica import centroid_dashboard

In [3]: os.makedirs('obs_test2')

In [4]: metrics_obsid, metrics_slot = centroid_dashboard.get_observed_metrics(25516, metrics_file='test_file2.dat')
2024-10-06 16:39:48,752 Running get_observed_att_errors for 25516
2024-10-06 16:40:30,823 Running get_observed_att_errors for 28127
2024-10-06 16:40:52,912 Dwell at 2024:275:07:25:10.363

In [5]: kwargs = {"factor": 20, "save": True}

In [6]: centroid_dashboard.plot_observed_metrics(25516, plot_dir='obs_test2', coord='dr', att_errors=metrics_obsid["att_errors"], **kwa
   ...: rgs)
2024-10-06 16:41:21,172 Writing plot file obs_test2/observed_drs_25516.png
2024-10-06 16:41:21,477 Writing plot file obs_test2/crs_time_25516.png
2024-10-06 16:41:23,568 Writing plot file obs_test2/crs_vis_25516.png
2024-10-06 16:41:24,726 Writing plot file obs_test2/n_kalman_25516.png

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I assume you meant "guide" not "acquisition" in the description.

@jeanconn
Copy link
Contributor Author

jeanconn commented Oct 8, 2024

No, this is related to https://github.com/sot/chandra_aca/pull/170/files so it is the acquisition mask in chandra_aca getting in via new code that is the issue, not the new change to exclude COLOR1 stars in guide.

Base automatically changed from omitted-missing-star to master October 24, 2024 18:50
@jeanconn jeanconn merged commit df423e8 into master Oct 24, 2024
2 checks passed
@jeanconn jeanconn deleted the add-color1-for-dash branch October 24, 2024 18:51
@javierggt javierggt mentioned this pull request Nov 7, 2024
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

Successfully merging this pull request may close these issues.

2 participants