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

JP-3488 Input files to outlier_detection accidentally deleted #8263

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

penaguerrero
Copy link
Contributor

@penaguerrero penaguerrero commented Feb 6, 2024

Resolves JP-3488

Closes #8128

This PR addresses unintentional deletion of tweakreg output files when they are used as input to the outlier_detection step.

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

@tapastro
Copy link
Contributor

tapastro commented Feb 6, 2024

This was bad code on my part - the current code expects the model filename to have suffix '_cal' - if any step saves before outlier_detection (or the input has a custom suffix) then the line defining outlr_file will not replace the suffix and just delete the input file. outlr_file should be changed to remove any suffix if present, then add the 'outlier_i2d.fits' suffix to it rather than replace.

Copy link

codecov bot commented Feb 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (12d4c91) 75.19% compared to head (82026d8) 75.20%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8263   +/-   ##
=======================================
  Coverage   75.19%   75.20%           
=======================================
  Files         470      470           
  Lines       38580    38581    +1     
=======================================
+ Hits        29012    29013    +1     
  Misses       9568     9568           
Flag Coverage Δ *Carryforward flag
nightly 77.38% <ø> (ø) Carriedforward from db8c48d

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

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

@hbushouse hbushouse added this to the Build 10.2 milestone Feb 13, 2024
CHANGES.rst Outdated Show resolved Hide resolved
@hbushouse hbushouse changed the title WIP: JP-3488 Input files to Image3Pipeline are deleted WIP: JP-3488 Input files to outlier_detection accidentally deleted Feb 13, 2024
@hbushouse
Copy link
Collaborator

Looks like this needs a regression test run. Also, has it been tested in the use case originally reported by the user?

@hbushouse
Copy link
Collaborator

Still WIP or is it ready to go? If the latter, please update the PR title.

penaguerrero and others added 2 commits February 13, 2024 11:59
Co-authored-by: Howard Bushouse <bushouse@stsci.edu>
@penaguerrero
Copy link
Contributor Author

I decided to leave the WIP because I do not have data to test this. I requested it and gave them the branch name to test it themselves but have not heard back

@penaguerrero penaguerrero changed the title WIP: JP-3488 Input files to outlier_detection accidentally deleted JP-3488 Input files to outlier_detection accidentally deleted Feb 14, 2024
@penaguerrero
Copy link
Contributor Author

Bryan Hilbert confirmed branch is working as expected. Ready for review.

@hbushouse
Copy link
Collaborator

Has a regression test run been executed? I don't see any references/links to it here.

@penaguerrero
Copy link
Contributor Author

had forgotten about it. Tests running at: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1207/

@hbushouse
Copy link
Collaborator

Regression test failures are unrelated (updated ref files and bad test inputs). So this looks good.

@hbushouse hbushouse merged commit d63fb5a into spacetelescope:master Feb 14, 2024
29 checks passed
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.

Input files to Image3Pipeline are deleted
3 participants