Added statistical test output to ocrreject_exam#197
Open
m-dallas wants to merge 4 commits into
Open
Conversation
sean-lockwood
requested changes
Jun 17, 2026
sean-lockwood
left a comment
Member
There was a problem hiding this comment.
Suggest making PEP8 changes as suggested via:
pycodestyle --ignore=E501,W503,W504 ocrreject_exam.py
I haven't tried running the tool yet.
Contributor
Author
|
Updated the script to comply with PEP8, I also double checked that the tool is giving the results we expect. |
sean-lockwood
approved these changes
Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Major improvements
• Renamed
avg_ratiotocombined_ratiosince the value isn’t really an average.• Added the new method
combined_ratio_threshold()which generates the highest combined_ratio possible under the null hypothesis that CRs are randomly distributed across the detector given a significance level (alpha) and a mean number of pixels each CR hit is expected to impact (cr_size). This was developed in response to feedback that there should be an included way for users to immediately see if a dataset was likely overflagged.• Added
overflagged_statto the results output. The value isTrueif the observedcombined_ratio>= to the threshold set bycombined_ratio_threshold(). Comparing to manually checked datasets from 2016-2025 shows this stat is agrees with the manual classification of the data ~93% of the time.Bug fixes:
• There was a bug in how the extraction box region was defined. Previously it would always extend 1 pixel lower than it should. This updated version is the same as what was shown in STIS ISR 2019-02 and ensures the correct extraction box size.
• There was a bug where subarray readouts wouldn’t properly render in the html files. This was fixed.
• Trying to only show figures in an interactive environment was buggy (especially with qt), so this was changed to the “
show_plot” argument which allows users to choose if they want the plots to render or just to be saved to a directory.The tests and docstrings have been updated to reflect these changes. This also shouldn’t break the plot rendering in the STIS notebooks but it’s worth double checking.