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

Allow None input to convert_time_format_spk #163

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Conversation

taldcroft
Copy link
Member

@taldcroft taldcroft commented Dec 10, 2023

Description

This fixes an issue in convert_time_format_spk where it ended up wrapping None as a numpy array with object dtype. The subsequent code in CxoTime then did not recognize that as None and failed.

Interface impacts

None

Testing

Unit tests

  • Mac (with a new test)
(ska3-perf) ➜  chandra_aca git:(fix-convert-time-spk) git rev-parse HEAD                    
22629cb62810d28a31804260edddfd19a7de8f1c
(ska3-perf) ➜  chandra_aca git:(fix-convert-time-spk) pytest            
================================================== test session starts ===================================================
platform darwin -- Python 3.10.8, pytest-7.2.1, pluggy-1.0.0
rootdir: /Users/aldcroft/git, configfile: pytest.ini
plugins: timeout-2.1.0, anyio-3.6.2
collected 198 items                                                                                                      

chandra_aca/tests/test_aca_image.py ..............                                                                 [  7%]
chandra_aca/tests/test_all.py ........................                                                             [ 19%]
chandra_aca/tests/test_attitude.py .............................................................                   [ 50%]
chandra_aca/tests/test_dark_model.py ....                                                                          [ 52%]
chandra_aca/tests/test_drift.py ..........................                                                         [ 65%]
chandra_aca/tests/test_maude_decom.py ...............                                                              [ 72%]
chandra_aca/tests/test_planets.py ..............                                                                   [ 79%]
chandra_aca/tests/test_psf.py ...                                                                                  [ 81%]
chandra_aca/tests/test_residuals.py ss...                                                                          [ 83%]
chandra_aca/tests/test_star_probs.py ................................                                              [100%]

============================================ 196 passed, 2 skipped in 42.56s =============================================

Independent check of unit tests by Jean

  • Linux

Functional tests

No functional testing. Just an FYI that this issue is not a problem in cxotime.convert_time_format:

In [1]: from cxotime import convert_time_format

In [2]: convert_time_format(None, "secs")
Out[2]: 818607256.278

@taldcroft taldcroft requested a review from jeanconn December 10, 2023 14:54
Copy link
Contributor

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

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

I added a newline so this would pass checks.

@taldcroft taldcroft merged commit d6be88f into master Dec 11, 2023
2 checks passed
@taldcroft taldcroft deleted the fix-convert-time-spk branch December 11, 2023 11:57
@javierggt javierggt mentioned this pull request Feb 6, 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