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 jump detection for stcal changes #723

Merged
merged 3 commits into from
Jun 27, 2023

Conversation

braingram
Copy link
Collaborator

@braingram braingram commented Jun 27, 2023

stcal recently updated jump detection and ramp fitting.
https://github.com/spacetelescope/stcal/releases/tag/1.4.0

This PR only addresses the jump detection changes (which changed the number of returned values).

The new returned values are ignored in this PR to fix the errors introduced by these changes.

I am not familiar with the arguments or return values for this function so a check for validity (and not just relying on a lack of failures) is needed.

Checklist

  • added entry in CHANGES.rst under the corresponding subsection
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)

Regression tests run (against stcal main which is now released):
https://plwishmaster.stsci.edu:8081/blue/organizations/jenkins/RT%2FRoman-Developers-Pull-Requests/detail/Roman-Developers-Pull-Requests/257/pipeline/206/

@github-actions github-actions bot added the jump label Jun 27, 2023
@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.02 ⚠️

Comparison is base (7d5d5fd) 75.72% compared to head (01a94a8) 75.70%.

❗ Current head 01a94a8 differs from pull request most recent head c35f3bf. Consider uploading reports for the commit c35f3bf to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #723      +/-   ##
==========================================
- Coverage   75.72%   75.70%   -0.02%     
==========================================
  Files          90       90              
  Lines        5478     5478              
==========================================
- Hits         4148     4147       -1     
- Misses       1330     1331       +1     
Flag Coverage Δ *Carryforward flag
nightly 64.54% <ø> (-0.01%) ⬇️ Carriedforward from f42b0de

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

Impacted Files Coverage Δ
romancal/jump/jump_step.py 96.05% <0.00%> (-1.32%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@braingram
Copy link
Collaborator Author

braingram commented Jun 27, 2023

@braingram braingram marked this pull request as ready for review June 27, 2023 19:16
@braingram braingram requested a review from a team as a code owner June 27, 2023 19:16
@braingram braingram requested a review from ddavis-stsci June 27, 2023 19:18
@ddavis-stsci ddavis-stsci added this to the 23Q4_B11 milestone Jun 27, 2023
Copy link
Collaborator

@ddavis-stsci ddavis-stsci left a comment

Choose a reason for hiding this comment

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

LGTM, if you could add the Roman-Developers-Pull-Requests link #257 to the checklist.
I'll take care of the ramp fit errors you are seeing.

@ddavis-stsci
Copy link
Collaborator

Can you also change pyproject.toml to pin 'stcal >=1.4.0',

@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Jun 27, 2023
@braingram
Copy link
Collaborator Author

Can you also change pyproject.toml to pin 'stcal >=1.4.0',

Thanks for taking a look at this and thanks for looking into the ramp fitting changes.

Anything I can do to help finish up this PR?

@ddavis-stsci
Copy link
Collaborator

I think we're good to go!

@braingram braingram enabled auto-merge June 27, 2023 19:43
@braingram braingram merged commit 8b2cdcc into spacetelescope:main Jun 27, 2023
@braingram braingram deleted the stcal_detect_jumps branch June 29, 2023 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file jump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants