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

Tickets/DM-39447 #186

Merged
merged 2 commits into from
May 31, 2023
Merged

Tickets/DM-39447 #186

merged 2 commits into from
May 31, 2023

Conversation

gmegh
Copy link
Collaborator

@gmegh gmegh commented May 30, 2023

Fixing filter names in testData fits files from "g" to the new "g_6".

For all the testData/gen3TestRepo fits files, the filter value was updated to comply with new obs_lsst ts8 translator convention. The update was performed with the following snippet:

from astropy.io import fits
ass =['R00_SW0', 'R00_SW1', 'R04_SW0', 'R04_SW1', 'R40_SW0', 'R40_SW1', 'R44_SW0', 'R44_SW1']
for sens in ass:
    filename = f'/sdf/home/g/gmegias/aos/ts_wep/tests/testData/gen3TestRepo/LSSTCam/raw/all/raw/20211231/MC_H_20211231_006000/raw_LSSTCam_g_MC_H_20211231_006000_{sens}_LSSTCam_raw_all.fits'
    hdul = fits.open(filename)  # open a FITS file
    hdul[0].header['FILTER'] = 'g_6'
    hdul.writeto(filename, overwrite=True)

@gmegh gmegh force-pushed the tickets/DM-39447 branch 3 times, most recently from 2b63d68 to 9b8dc72 Compare May 30, 2023 23:07
@gmegh gmegh requested review from jbkalmbach and suberlak May 30, 2023 23:29
@suberlak
Copy link
Contributor

Hi! Since all checks have passed I see that the update works. But just for reference, if you could paste here the code that you used to make the updates, it would make it useful to keep track of what happened. Also, please make sure to update the versionHistory file, and tag the develop branch after merger. Thanks!

Copy link
Contributor

@suberlak suberlak left a comment

Choose a reason for hiding this comment

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

Thanks for the update, made a few small comments in conversation and the empty file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file meant to be there?

@jbkalmbach
Copy link
Member

jbkalmbach commented May 31, 2023

Hi! Since all checks have passed I see that the update works. But just for reference, if you could paste here the code that you used to make the updates, it would make it useful to keep track of what happened. Also, please make sure to update the versionHistory file, and tag the develop branch after merger. Thanks!

Tags should be made on the main branch after merging the develop branch into main with a no fast-forward merge. If you haven't done that before let me know and I'll walk you through it.

Copy link
Member

@jbkalmbach jbkalmbach left a comment

Choose a reason for hiding this comment

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

What happens when you run butler query-datasets tests/testData/gen3TestRepo/? When I do it the physical_filter of all the data is still 'g' and not 'g_6' as I think it should be? Or is that not right?

@jbkalmbach jbkalmbach self-requested a review May 31, 2023 19:09
Copy link
Member

@jbkalmbach jbkalmbach left a comment

Choose a reason for hiding this comment

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

We can leave the database as is for now since these tests are passing and fighting the butler isn't the point of these tests. Just want us to keep in mind this disparity between the butler database and the actual headers of our fits files in case it comes back to bite us later.

@gmegh
Copy link
Collaborator Author

gmegh commented May 31, 2023

Perfect! Thanks to both, @jbkalmbach and @suberlak . Merging

@gmegh gmegh merged commit 7aea804 into develop May 31, 2023
@jbkalmbach jbkalmbach deleted the tickets/DM-39447 branch June 7, 2023 21:49
jbkalmbach pushed a commit that referenced this pull request Jun 22, 2023
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.

3 participants