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

Update cube and cutout unittests #90

Merged
merged 11 commits into from
Aug 17, 2023
Merged

Update cube and cutout unittests #90

merged 11 commits into from
Aug 17, 2023

Conversation

hmlewis-astro
Copy link
Contributor

Updates to existing unit tests and adding new tests to account for changes in CutoutFactory (made in #88). Tests verify that cutouts are being made correctly from the new TICA cubes (without error arrays) and verify known differences in TICA vs. SPOC FFIs, cubes, and cutouts.

Note, there is a placeholder unittest (test_cube_cut.py::test_tica_cutout_error) to remind us to go back to look at whether we can/should point the test to an existing TICA cube (one that does have the error array and will be among the last cubes to me re-built, e.g., Sector 27) that will be re-built later, to verify that cutouts can still be made from cubes with error array.

@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (fe56d85) 94.45% compared to head (92a65ed) 94.45%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #90   +/-   ##
=======================================
  Coverage   94.45%   94.45%           
=======================================
  Files           8        8           
  Lines        1443     1443           
=======================================
  Hits         1363     1363           
  Misses         80       80           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jaymedina
Copy link
Collaborator

jaymedina commented Aug 10, 2023

Thanks for this PR - I thought about it a bit more about testing the new CutoutFactory changes against an older TICA cube (still with the error array) and I think you can just use a SPOC cube with the TICA header keywords superimposed. Before the removal of the error arrays, the TICA cubes were structurally identical to the SPOC ones, just with different header keywords. If we go this route, your code will look like this:

def test_tica_cutout_error():
     tica_cube_error = CubeFactory() # this is actually a SPOC cube but we're gonna update the headers with TICA 
     tica_cube_no_error = TicaCubeFactory()
     
     with fits.open(tica_cube_error, mode='u') as hdulist:
           tica_cube_error[0].header = fits.getheader(tica_cube_no_error, 0)
           tica_cube_error[1].header = fits.getheader(tica_cube_no_error, 1)
           
           etc...
     # now do the actual testing 
     out_file = CutoutFactory(tica_cube_error)

The logic here isn't exact, I would verify with the astropy documentation the best way to replace one HDU lists headers with another, but this is more or less the concept of what I think will be the easiest path forward for this unit test in particular. And we don't need to be accurate with the WCS since this unit test is just to make sure the old TICA cube HDU list structure still works.

@hmlewis-astro hmlewis-astro marked this pull request as ready for review August 11, 2023 19:46
falkben
falkben previously approved these changes Aug 14, 2023
Copy link
Member

@falkben falkben left a comment

Choose a reason for hiding this comment

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

I will leave it to others to decide if this tested all the various header differences, but these tests look good to me.

jaymedina
jaymedina previously approved these changes Aug 16, 2023
Copy link
Collaborator

@jaymedina jaymedina left a comment

Choose a reason for hiding this comment

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

These tests look good to me! Just had one question on an assertion re: the CADENCEN0 header keyword for TICA. Other than that, I think this is ready for merge.

astrocut/tests/test_cube_cut.py Outdated Show resolved Hide resolved
@jaymedina jaymedina merged commit 061cd6b into main Aug 17, 2023
7 of 8 checks passed
@jaymedina jaymedina deleted the update-cube-unittests branch August 17, 2023 15:40
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