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

DM-26943: Use AIRMASS or AMSTART headers for imsim if they exist #255

Merged
merged 2 commits into from Sep 29, 2020

Conversation

timj
Copy link
Member

@timj timj commented Sep 25, 2020

No description provided.

Copy link

@ehneilsen ehneilsen left a comment

Choose a reason for hiding this comment

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

I would expect the _erfa import to be at the head of the file with the rest of the imports. Is there a reason it is embedded in the code?
Also, it looks like astropy._erfa has now been deprecated by the astropy team in favor of using a separate standalone pyerfa package. It doesn't look like this is available yet in the version of astropy used here, though.

dec = Angle(self._header["DECTEL"], unit=u.deg)

# Use erfa directly
import astropy._erfa as erfa

Choose a reason for hiding this comment

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

The import should maybe be moved to the top of the file?

@timj
Copy link
Member Author

timj commented Sep 28, 2020

Thanks. Yes, I'll try to support pyerfa (I found out about that from the astropy slack over the weekend). I'll also probably move the core hour angle to azel code into astro_metadata_translator since it's generic.

This is faster and more reliable than trying to derive it
from the RA/Dec in the future (which also requires assumptions
on leap seconds).
Copy link

@ehneilsen ehneilsen 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; hopefully this will speed things up considerably.

@timj timj merged commit 5865270 into master Sep 29, 2020
@timj timj deleted the tickets/DM-26943 branch September 29, 2020 21:11
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.

None yet

2 participants