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

HlA-1059: Reference catalog weights corrected #1616

Conversation

s-goldman
Copy link
Collaborator

This PR corrects the weighting of the reference catalogs; the code now uses 1/sigma**2. The important lines of code are 283 and 284. I also added line 281 to avoid a possible divide by zero error. The remaining changes are the result of code formatting (black).

@s-goldman s-goldman requested review from mdlpstsci and a team as code owners July 17, 2023 18:29
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Patch coverage: 67.44% and project coverage change: +0.02 🎉

Comparison is base (00022f6) 32.36% compared to head (ea368d5) 32.39%.

❗ Current head ea368d5 differs from pull request most recent head 451672d. Consider uploading reports for the commit 451672d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1616      +/-   ##
==========================================
+ Coverage   32.36%   32.39%   +0.02%     
==========================================
  Files         159      159              
  Lines       35053    35056       +3     
==========================================
+ Hits        11345    11355      +10     
+ Misses      23708    23701       -7     
Impacted Files Coverage Δ
drizzlepac/haputils/config_utils.py 68.54% <0.00%> (-0.46%) ⬇️
drizzlepac/haputils/product.py 66.07% <68.63%> (+0.05%) ⬆️

... and 3 files with indirect coverage changes

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

@s-goldman s-goldman added the jirahub-ignore Avoid linking to Jira label Jul 17, 2023
Copy link
Collaborator

@mdlpstsci mdlpstsci left a comment

Choose a reason for hiding this comment

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

Since SCSB only has a general set of standards (https://innerspace.stsci.edu/display/SCSB/Code+standards+refresher), and not a specific package we are supposed to use, I have tried to keep my opinion to myself. However, there are a few places where I found the Black code updates to make the code harder to parse. Please address these few items.

drizzlepac/haputils/product.py Outdated Show resolved Hide resolved
drizzlepac/haputils/product.py Outdated Show resolved Hide resolved
drizzlepac/haputils/product.py Outdated Show resolved Hide resolved
drizzlepac/haputils/product.py Outdated Show resolved Hide resolved
drizzlepac/haputils/product.py Outdated Show resolved Hide resolved
@s-goldman s-goldman changed the title Hla 1059 reference catalog weights are incorrect HlA-1059: Reference catalog weights corrected Jul 17, 2023
@mdlpstsci
Copy link
Collaborator

@s-goldman Where is the update to the "wfpc2_wfpc2_index.json"?

@mdlpstsci
Copy link
Collaborator

@s-goldman Oops. I was checking both your PRs and made a comment in the wrong PR. The change to the parameter index file needs to be in PR#1052 and not this PR for proper tracking of history. Please remove the changes for config_utils.py from this PR and put into PR#1052. Once that is done, this PR is ready to go.

@s-goldman
Copy link
Collaborator Author

@s-goldman Oops. I was checking both your PRs and made a comment in the wrong PR. The change to the parameter index file needs to be in PR#1052 and not this PR for proper tracking of history. Please remove the changes for config_utils.py from this PR and put into PR#1052. Once that is done, this PR is ready to go.

I'm sorry, I should have caught that. The change to config_utils is now in the HLA-1052 PR.

Copy link
Collaborator

@mdlpstsci mdlpstsci left a comment

Choose a reason for hiding this comment

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

Thanks for all the fixes!

@s-goldman s-goldman merged commit 3643768 into spacetelescope:master Jul 19, 2023
11 checks passed
@s-goldman s-goldman deleted the hla-1059_reference_catalog_weights_are_incorrect branch July 19, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jirahub-ignore Avoid linking to Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants