Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Suppress erfa warnings for future times #138

Merged
merged 3 commits into from
Mar 22, 2023

Conversation

bmorris3
Copy link
Contributor

@bmorris3 bmorris3 commented Mar 21, 2023

Methods for synthetic datamodels like create_wfi_image generate metadata (create_meta) with exposures (create_exposure). Exposures contain times represented with astropy.time.Time objects, which have limited support for times sufficiently far in the future. This stems from uncertainty due to upcoming leap seconds, which may (or may not) be added between now and far-ish future times. Far-ish future times can be initialized without a problem in some formats/scales, but if you translate them to some other formats, you get an ERFA warning about a "dubious year", like this:

In [1]: from astropy.time import Time

In [2]: Time(1893456000, format='unix')  # no problems
Out[2]: <Time object: scale='utc' format='unix' value=1893456000.0>

In [3]: Time(1893456000, format='unix').iso  # problems
/Users/bmmorris/miniconda3/envs/jdaviz-only/lib/python3.10/site-packages/erfa/core.py:154: ErfaWarning: ERFA function "d2dtf" yielded 1 of "dubious year (Note 5)"
  warnings.warn('ERFA function "{}" yielded {}'.format(func_name, wmsg),
Out[3]: '2030-01-01 00:00:00.000'

The unix time chosen in the example above is the upper limit on the randomly drawn times used by roman_datamodels:

def generate_utc_timestamp():
# Random timestamp between 2020-01-01 and 2030-01-01
return generate_float(1577836800.0, 1893456000.0)

so the later randomly drawn times within the range will throw an erfa warning. We can dodge the warning by suppressing it directly or by changing the time range. Since the time range is meant to mimic Roman's mission duration, I opted for the former, and put in a switch (ignore_erfa_warnings) to recover the old behavior.

This PR is in support of spacetelescope/jdaviz#1822.

Copy link
Collaborator

@WilliamJamieson WilliamJamieson left a comment

Choose a reason for hiding this comment

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

These changes look good to me. However, do you mind adding a changelog entry for this.

@WilliamJamieson
Copy link
Collaborator

Note, this is part of a solution to #129.

@bmorris3 bmorris3 force-pushed the suppress-erfa-warning branch from cda4d45 to 7f84b9b Compare March 22, 2023 14:25
Copy link
Collaborator

@PaulHuwe PaulHuwe left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Patch coverage: 87.50% and project coverage change: +0.01 🎉

Comparison is base (086dd88) 76.88% compared to head (cda4d45) 76.89%.

❗ Current head cda4d45 differs from pull request most recent head 7f84b9b. Consider uploading reports for the commit 7f84b9b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #138      +/-   ##
==========================================
+ Coverage   76.88%   76.89%   +0.01%     
==========================================
  Files          17       17              
  Lines        2167     2173       +6     
==========================================
+ Hits         1666     1671       +5     
- Misses        501      502       +1     
Impacted Files Coverage Δ
src/roman_datamodels/random_utils.py 95.37% <87.50%> (-0.71%) ⬇️

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@WilliamJamieson WilliamJamieson merged commit 1d1876d into spacetelescope:main Mar 22, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants