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

write selected LSST Stack package versions and tags to FITS primary header #232

Merged
merged 5 commits into from
Oct 22, 2019

Conversation

jchiang87
Copy link
Collaborator

No description provided.

@coveralls
Copy link

coveralls commented Oct 1, 2019

Coverage Status

Coverage increased (+0.2%) to 72.951% when pulling 31d7c2f on issue_231/write_stack_versions_and_tags into d1eac95 on master.

@jchiang87
Copy link
Collaborator Author

@cwwalter This is ready to go.

Copy link
Member

@cwwalter cwwalter left a comment

Choose a reason for hiding this comment

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

OK this looks good to me. Two things I noticed you could consider:

  • This was actually already true before this change for the imSim version number, but we are only going to see these keywords on the electronics readout versions. Would it be good to (eventually?) do it in the e-image and then would they automatically be copied over to the version during readout? I think as it is, e-image files will have no provenance. If we only view them as debugging items maybe that is OK, but if we ever stored them as a base we might miss it.

  • The test doesn't check the tag vs version capability you build into the code and are using.

@jchiang87
Copy link
Collaborator Author

Unfortunately, getting this info into the eimage is difficult since the FITS file writing is done by the galsim Image.write function and there's no direct interface to manipulating the header keywords before writing. If we really want this, we'd need to update the file after it is first written.

@cwwalter
Copy link
Member

Unfortunately, getting this info into the eimage is difficult since the FITS file writing is done by the galsim Image.write function and there's no direct interface to manipulating the header keywords before writing. If we really want this, we'd need to update the file after it is first written.

Ah, OK; too bad. Actually I think you already told me this in the past.

@rmjarvis
Copy link
Contributor

Actually, anything you put into image.header should show up in the written FITS header.

@rmjarvis
Copy link
Contributor

I don't think there's particularly good documentation for this feature, but this unit test should show you the syntax if you want to try it.

https://github.com/GalSim-developers/GalSim/blob/releases/2.2/tests/test_image.py#L2412

@jchiang87
Copy link
Collaborator Author

Thanks, Mike. I've updated the code to add the version keywords to the eimage files following the galsim example, and I refactored the version keyword handling so that we can test some of that functionality directly.

Unless I hear otherwise, I will merge this into master at 5pm PT today, and will tag the imSim release for Run2.2i.

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

4 participants