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

Revert removal of ramp_fit code to stcal #6119

Merged
merged 4 commits into from
Jun 7, 2021

Conversation

jdavies-st
Copy link
Collaborator

@jdavies-st jdavies-st commented Jun 4, 2021

This PR reverts the removal of the ramp fitting code (to stcal). Specifically it reverts commits 85feb11 and 5a9aaf8 but retains the bugfix and test updates in bfe9d64 .

This also turns on the 2 ramp_fit multiprocessing unit tests. To make sure that aspect of the code is no longer broken.

Regression tests run on this branch here and pass.

https://plwishmaster.stsci.edu:8081/job/RT/job/jdavies-dev/251/

Note that the test failure is an intermittent one that @hbushouse warned me about that occasionally pops up in our regression tests due to cosmic variance.

Checklist

  • Tests
  • Documentation
  • Change log
  • Milestone
  • Label(s)

@jdavies-st jdavies-st added this to the Build 7.8 milestone Jun 4, 2021
@codecov
Copy link

codecov bot commented Jun 4, 2021

Codecov Report

Merging #6119 (0564070) into 1.2.x (0a692e8) will increase coverage by 0.50%.
The diff coverage is 76.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##            1.2.x    #6119      +/-   ##
==========================================
+ Coverage   77.15%   77.65%   +0.50%     
==========================================
  Files         402      406       +4     
  Lines       34360    36128    +1768     
==========================================
+ Hits        26509    28057    +1548     
- Misses       7851     8071     +220     
Flag Coverage Δ *Carryforward flag
nightly 77.15% <100.00%> (ø) Carriedforward from 0a692e8
unit 56.69% <76.38%> (+1.50%) ⬆️

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

Impacted Files Coverage Δ
jwst/ramp_fitting/ols_fit.py 96.63% <ø> (ø)
jwst/ramp_fitting/ramp_fit_step.py 76.06% <53.33%> (-15.98%) ⬇️
jwst/ramp_fitting/gls_fit.py 59.68% <59.68%> (ø)
jwst/ramp_fitting/utils.py 96.07% <96.07%> (ø)
jwst/ramp_fitting/ramp_fit.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a692e8...0564070. Read the comment docs.

Copy link
Contributor

@kmacdonald-stsci kmacdonald-stsci left a comment

Choose a reason for hiding this comment

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

This looks good and has the JP-1950 bug fix.

@jdavies-st jdavies-st changed the base branch from master to 1.2.x June 6, 2021 17:06
@jdavies-st jdavies-st force-pushed the revert-rampfit-to-stcal branch from bad885a to 0564070 Compare June 7, 2021 00:17
@jdavies-st
Copy link
Collaborator Author

FYI @kmacdonald-stsci I'm just going to do the revert here on the release branch so only the release version jwst 1.2.1 will have this. jwst master branch will still have all your changes and you can carry on development as you were. 👍

@jdavies-st jdavies-st merged commit d75e47e into spacetelescope:1.2.x Jun 7, 2021
@jdavies-st jdavies-st deleted the revert-rampfit-to-stcal branch June 7, 2021 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants