JP-2645 - Snowball and Shower flagging in Jump

[mwregan2]

pass parameters for snowball showers

Resolves JP-2645

Closes #

This PR addresses ...

Checklist for maintainers

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

[mwregan2]

I can't figure out why the unit tests are failing. They work for me. The assert error is saying something about file (s) not being closed. It is referring to the gain and read noise files.

[cshanahan1]

I see that you added some tests in stcal for some of the functions you added there, but I think it would be good to have a few tests in JWST as well for the full step using these new options

[cshanahan1]

@mwregan2 (summarizing some of our call earlier)

i added a test to demonstrate the use of the sat_required_snowball flag, and it doesn't seem to be working (maybe an error on my end?). what i did was add in circles of pixels in the 1st (not 0th) read of the input data. inside those circles, and in every read after, there is a smaller circle of saturation flags in the core in the dq array in the input model. from what i understand of the code, what should be happening here is that the clusters of jump flags should be flagged as snowballs only if they also have saturated pixels in a circular shape within the boundaries, which they do in this case, so the jump flags should be expanded. it doesn't detect the snowballs in that read and therefore does not expand the radius of the jump dq flags as expected. additionally, it does detect a 'snowball' in the last read (where there is none). while i don't totally understand what is going on, and again it might just be an error on my end, i think it might have something to do with the calculation of the new_flagged_pixels array on line 349 in stcal.

for now i have commented this test out

[codecov[bot]]

Codecov Report

Base: 79.51% // Head: 79.66% // Increases project coverage by +0.14% :tada:

Coverage data is based on head (25f2950) compared to base (966f05d). Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7039      +/-   ##
==========================================
+ Coverage   79.51%   79.66%   +0.14%     
==========================================
  Files         412      412              
  Lines       37322    37356      +34     
==========================================
+ Hits        29678    29758      +80     
+ Misses       7644     7598      -46     

Flag	Coverage Δ		*Carryforward flag
nightly	79.71% <100.00%> (+0.21%)	:arrow_up:	Carriedforward from 8473256
unit	51.99% <100.00%> (+<0.01%)	:arrow_up:

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
jwst/jump/jump.py	100.00% <100.00%> (ø)
jwst/jump/jump_step.py	100.00% <100.00%> (ø)
jwst/regtest/regtestdata.py	86.23% <0.00%> (+1.37%)	:arrow_up:
jwst/regtest/conftest.py	88.04% <0.00%> (+23.36%)	:arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[hbushouse]

I'm assuming that there's no real benefit in running a regtest against the PR branch, because the new stuff is turned off by default, hence we don't expect the new code to get exercised at all. Given that, I'm thinking we're ready to merge. Any objections?

[mwregan2]

Yes, this will be transparent until we add the parameter reference files.

[hbushouse]

Full regression test run on the master branch after merging this PR was clean, as expected (no changes).

[mwregan2]

The tests were failing because files were not being closed. I just added the close to get the existing tests to work. I have no idea what changed to require that files be closed.

From: Clare Shanahan @.> Reply-To: spacetelescope/jwst @.> Date: Wednesday, October 5, 2022 at 3:28 PM To: spacetelescope/jwst @.> Cc: Michael Regan @.>, Author @.***> Subject: Re: [spacetelescope/jwst] JP-2645 - Snowball and Shower flagging in Jump (PR #7039)

@cshanahan1 commented on this pull request.

---

In jwst/jump/tests/test_jump_step.pyhttps://urldefense.com/v3/__https:/github.com/spacetelescope/jwst/pull/7039*discussion_r988270460__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!21jzkLU4neZF2_rgqTv6-tJwerwwXgFElihdguVOHjE7KStWNiepU8Uu8DTxuxKvxBB3gAp7Nu1_FuWPvXnA698$:

@@ -27,6 +27,7 @@ def generate_miri_reffiles(tmpdir_factory):

 gain_model.meta.subarray.xsize = xsize

 gain_model.meta.subarray.ysize = ysize

 gain_model.save(gainfile)

• gain_model.close()

they are written out to one of the pytest directories. i think you can override a reference file with a datamodel instead of a file path right, so these wouldn't have to be written out? i can open another pr to change this if thats the case

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/spacetelescope/jwst/pull/7039*discussion_r988270460__;Iw!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!21jzkLU4neZF2_rgqTv6-tJwerwwXgFElihdguVOHjE7KStWNiepU8Uu8DTxuxKvxBB3gAp7Nu1_FuWPvXnA698$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AF6KJLXLNYIJNIF3EV53Y3LWBXJFBANCNFSM6AAAAAAQJ7T5SA__;!!CrWY41Z8OgsX0i-WU-0LuAcUu2o!21jzkLU4neZF2_rgqTv6-tJwerwwXgFElihdguVOHjE7KStWNiepU8Uu8DTxuxKvxBB3gAp7Nu1_FuWPG9HC0KA$. You are receiving this because you authored the thread.Message ID: @.***>