JP-3763: Fix crash combining full and subarray for background

[thomaswilliamsastro]

Resolves JP-3763

This PR addresses combining full and subarray observations to accumulate a background. This fixes an issue introduced with #8326, where if you try to use a full frame observation to create a background for a subarray observation, it would crash. This instead uses the best of both worlds: if a multi-int exposure, it will use NINTS from the background obs to form a mean background. If you're mismatching full/subarray observations, it'll use the x/y size of the science to avoid array size mismatches.

I can't run regression tests, but have verified it works in my case

Tasks

• add an entry to CHANGES.rst within the relevant release section (otherwise add the no-changelog-entry-needed label to this PR)
• update or add relevant tests
• update relevant docstrings and / or docs/ page
• start a regression test and include a link to the running job (click here for instructions)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.89%. Comparing base (e860360) to head (5868435).
Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8787      +/-   ##
==========================================
+ Coverage   61.86%   61.89%   +0.03%     
==========================================
  Files         377      377              
  Lines       38911    38954      +43     
==========================================
+ Hits        24071    24110      +39     
- Misses      14840    14844       +4     

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

[melanieclarke]

Thanks for sending this in! Do you have some test data you can point me to so I can try out your fix?

Never mind! I found your original ticket for this issue (#7807) with your example program.

[thomaswilliamsastro]

@melanieclarke Yep, that's exactly the one :)

[melanieclarke]

@thomaswilliamsastro - thanks for confirming! I'm looking through the associations for that program, but I don't see one that specifies a background member for the SUB256 data. Did I miss one, or do you use a custom association for this task?

[thomaswilliamsastro]

@melanieclarke I used a custom association, so all the M33 data with "SKY" in the name is the background, and is associated to anything that doesn't have that

[melanieclarke]

Thanks, I got it now and I'm able to reproduce the problem.

I think this could use either a unit test or a regression test, to make sure we don't break your use case again. Would you like to try adding a unit test to cover your changes? If you don't have the bandwidth, I can take a look.

[thomaswilliamsastro]

I'm not well-versed in unit tests, so I'm very happy to leave it with you!

[melanieclarke]

I sent you a PR to your repo with some unit tests, and a few suggested fixes:
thomaswilliamsastro#1

[thomaswilliamsastro]

I've merged, rebased onto main, and pushed

[melanieclarke]

Thanks! This looks good to me now, but I'm going to tag in another reviewer to take a quick look, since I contributed some tests.

[emolter]

Looks good to me. Starting regression tests here and I'd approve this assuming no unexpected failures. My two questions are only minor

[melanieclarke]

Looks good to me. Starting regression tests here and I'd approve this assuming no unexpected failures. My two questions are only minor

Thanks Ned, I forgot I hadn't run a full set of regression tests yet!

Missed regression test checks

[melanieclarke]

No failures in regtests.

[melanieclarke]

@emolter - are you happy with this one now?