Fix a typo in `load_custom_wcs`

[izkgao]

This PR fixes a typo in load_custom_wcs from array_shape to pixel_shape.

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

• [x] added entry in CHANGES.rst within the relevant release section
• [x] updated or added relevant tests
• [ ] updated relevant documentation
• [x] added relevant milestone
• [x] added relevant label(s)
• [x] ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR
• [x] All comments are resolved
• [ ] Make sure the JIRA ticket is resolved properly

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 60.36%. Comparing base (84e1e94) to head (23dff80). Report is 2 commits behind head on master.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #8698 +/- ## ========================================== + Coverage 60.35% 60.36% +0.01% ========================================== Files 372 372 Lines 38360 38369 +9 ========================================== + Hits 23151 23163 +12 + Misses 15209 15206 -3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[melanieclarke]

Regression tests are here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1640/

[melanieclarke]

Regression tests are clean, but quick local tests show that this code still doesn't seem to work to load a custom wcs with a pixel_shape but no bounding box, even with this change. I think the issue is that it's looking for the pixel_shape in the top-level ASDF structure, instead of the wcs structure. A better fix might be to just delete lines 179-181, so that if pixel_shape is set in the wcs structure, it's not overwritten with None from the top-level structure.

Do you have a custom WCS you are testing with?

[izkgao]

I do have some custom WCS files that store array_shape and pixel_area in the top-level ASDF structure. This is because I ran a problem where pixel_area (and perhaps array_shape in older ASDF version?) did not show up in the saved ASDF file. So my solution was to store these values at the top level.

This is the code for saving custom WCS.

af = asdf.AsdfFile({'wcs': output_wcs, 'array_shape': output_wcs.array_shape, 'pixel_area': output_wcs.pixel_area})
af.write_to('output_wcs.asdf')

The main reason I store pixel_area is to save some time in not having to recalculate the pixel area.

https://github.com/spacetelescope/jwst/blob/4ecb40c57aaf1ae99fd6d8d9be66b7530277cf40/jwst/resample/resample.py#L114-L115

I have checked that only pixel_area is gone after saving to an ASDF file, but not array_shape and pixel_shape. So deleting line 180-181 should be fine, but keeping line 179 is necessary because the code above if output_wcs.pixel_area is None (line 114 in resample.py) accesses the pixel_area attribute of output_wcs without checking if it has the attribute. So this attribute needs to be set before going into that line.

https://github.com/spacetelescope/jwst/blob/4ecb40c57aaf1ae99fd6d8d9be66b7530277cf40/jwst/resample/resample_step.py#L177-L181

In case a future version of ASDF supports keeping pixel_area in the saved file. I suggest adding a line to check if it has a pixel_area attribute so it is not overwritten.

[melanieclarke]

I see, thanks for explaining how you're using those values. Your example explains something I have been wondering about how pixel_area ever gets set!

In that case, maybe we should support all three top-level values, but check for None, and don't overwrite the value in the wcs if it already exists?

[izkgao]

Sounds good. Now it checks if a value in the WCS is None or there is no such attribute, then uses the value in the top-level ASDF structure (if not there, then None).

[melanieclarke]

That looks great, thank you!

I think this could use a unit test or two to verify that the values now get passed in as expected under various conditions. I've been working on the unit tests for this module recently, so I can work on that and send you a PR on this branch, if that's okay?

Edited to add a link to the PR: https://github.com/izkgao/jwst/pull/2

[izkgao]

Looks great. I learned a lot from the unit tests you wrote. Thank you so much.

[melanieclarke]

Excellent, thanks for reviewing! I'll run regression tests again and assign somebody else to do a last review.

[melanieclarke]

Regression tests show no errors. @tapastro - can you please review?

Tests are here: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1642/

[melanieclarke]

@izkgao - I'll be out of contact for a bit, so this may go to the back burner. I'll check back in on this PR if it's not resolved by the time I return. Thanks for all the good work so far!

[izkgao]

@melanieclarke No problem. I also added another unit test for load_custom_wcs that checks if array_shape and pixel_shape are set correctly under the condition that only one of pixel_shape, array_shape or bounding_box is not None.