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

avoid linear interpolation between WFE values #834

Merged
merged 1 commit into from May 14, 2024

Conversation

mperrin
Copy link
Collaborator

@mperrin mperrin commented May 2, 2024

As discussed on slack with @tracy-beck and Matt Lallo, it's better for the WFE histogram not to interpolate between points when inferring WFE at times in between our measurements.

Before this PR:
Unknown-13

With this PR:

Unknown-12

@mperrin mperrin added the bug Something isn't working label May 2, 2024
@mperrin mperrin requested a review from tracy-beck May 2, 2024 14:42
@mperrin mperrin self-assigned this May 2, 2024
@pep8speaks
Copy link

pep8speaks commented May 2, 2024

Hello @mperrin, Thank you for updating !

Line 93:9: E265 block comment should start with '# '
Line 95:9: E265 block comment should start with '# '
Line 96:9: E265 block comment should start with '# '
Line 102:9: E265 block comment should start with '# '
Line 301:25: E712 comparison to False should be 'if cond is False:' or 'if not cond:'
Line 303:27: E712 comparison to True should be 'if cond is True:' or 'if cond:'
Line 384:35: E203 whitespace before ':'
Line 463:1: E266 too many leading '#' for block comment
Line 700:5: E266 too many leading '#' for block comment
Line 715:5: E266 too many leading '#' for block comment
Line 762:5: E266 too many leading '#' for block comment
Line 785:5: E266 too many leading '#' for block comment
Line 945:5: E266 too many leading '#' for block comment
Line 1032:49: W605 invalid escape sequence '\D'
Line 1068:1: E266 too many leading '#' for block comment
Line 1097:43: E203 whitespace before ':'
Line 1187:5: E722 do not use bare 'except'
Line 1340:5: E266 too many leading '#' for block comment
Line 1364:5: E266 too many leading '#' for block comment
Line 1384:22: W605 invalid escape sequence '\D'
Line 1703:126: E501 line too long (146 > 125 characters)
Line 1733:5: E731 do not assign a lambda expression, use a def
Line 2021:25: W605 invalid escape sequence '\D'
Line 2029:1: E266 too many leading '#' for block comment
Line 2108:9: E266 too many leading '#' for block comment
Line 2134:126: E501 line too long (142 > 125 characters)
Line 2145:13: E266 too many leading '#' for block comment
Line 2178:9: E266 too many leading '#' for block comment

Comment last updated at 2024-05-12 04:10:35 UTC

@mperrin
Copy link
Collaborator Author

mperrin commented May 7, 2024

Ping to @tracy-beck, would you have a moment to review this GitHub PR for the histogram fix that I chatted about with you and Matt? Thanks

@mperrin
Copy link
Collaborator Author

mperrin commented May 13, 2024

@kulpster85 might you be available to review this PR, perhaps? It's pretty straightforward I think. I had asked Tracy (who had originally pointed out this bug to me along with Matt L.) but it sounds like she may not be available to review this PR due to other things on her plate.

@kulpster85
Copy link
Contributor

@mperrin yes I can take a look

Copy link
Contributor

@kulpster85 kulpster85 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@mperrin
Copy link
Collaborator Author

mperrin commented May 14, 2024

@kulpster85 approved this PR. Despite that, Github is for some reason still showing "Review Required, at least 1 approving review is still required". Rather than spend time debugging why Github is confused, I'm going to simply merge anyway despite that warning.

@mperrin mperrin merged commit 70a678a into spacetelescope:develop May 14, 2024
7 checks passed
@mperrin mperrin deleted the fix_trending_histogram branch May 14, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants