Skip to content

Ensure we check leap seconds only when Time is actually used.#9491

Merged
mhvk merged 1 commit intoastropy:masterfrom
mhvk:iers-leap-second-import-order
Oct 28, 2019
Merged

Ensure we check leap seconds only when Time is actually used.#9491
mhvk merged 1 commit intoastropy:masterfrom
mhvk:iers-leap-second-import-order

Conversation

@mhvk
Copy link
Contributor

@mhvk mhvk commented Oct 27, 2019

I went with only calling update_leap_seconds on first call of Time. Somewhat ugly but it does work. Better suggestions most welcome...

fixes #9490

@mhvk mhvk added time Bug Affects-dev PRs and issues that do not impact an existing Astropy release erfa utils.iers labels Oct 27, 2019
@mhvk mhvk added this to the v4.0 milestone Oct 27, 2019
@mhvk mhvk requested review from eteq and taldcroft October 27, 2019 15:40
# Because of import problems, this can only be done on
# first call of Time.
global _LEAP_SECONDS_CHECKED
if not _LEAP_SECONDS_CHECKED:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could save two lines of code and a global by holding this attribute in the Time class. So

if not getattr(Time, '_LEAP_SECONDS_CHECKED', False):
    Time._LEAP_SECONDS_CHECKED = True
    update_leap_seconds()

But on the whole this solution seems fine. I did confirm that it resolves the problems I found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about putting it on the class, but ended up feeling it was slightly more elegant (less inelegant?) to have it as global state,. Also perhaps slightly faster?

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.

LGTM.

@mhvk mhvk merged commit 9e60175 into astropy:master Oct 28, 2019
@mhvk mhvk deleted the iers-leap-second-import-order branch October 28, 2019 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Affects-dev PRs and issues that do not impact an existing Astropy release Bug erfa time utils.iers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ordering of leap second update can lead to import warnings

2 participants