Skip to content

Add the option to subtract histograms to the merge_histogram function#793

Merged
mike-w-wilson merged 12 commits intomainfrom
mw/merge_hist_diff
Jul 31, 2025
Merged

Add the option to subtract histograms to the merge_histogram function#793
mike-w-wilson merged 12 commits intomainfrom
mw/merge_hist_diff

Conversation

@mike-w-wilson
Copy link
Copy Markdown
Contributor

This adds the option to subtract histograms in the merge_histograms function as well as tests for the entire function

Copy link
Copy Markdown
Contributor

@ch-kr ch-kr 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 adding! one docstring suggestion I forgot to add as a commit and a minor code refactoring suggestion

Comment thread gnomad/utils/annotations.py Outdated
@mike-w-wilson mike-w-wilson requested a review from ch-kr July 31, 2025 13:43
@mike-w-wilson
Copy link
Copy Markdown
Contributor Author

Back to you, @ch-kr -- I refactored again. I agree on putting the negative value handling into a private function but I found it a bit confusing that we were _handle_negative_values_for_diff even on sum operations so I pulled it back out. I know the logic made sense but I find it easier to follow this way.

Copy link
Copy Markdown
Contributor

@ch-kr ch-kr left a comment

Choose a reason for hiding this comment

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

I removed the if and added a couple extra tests -- I think this is good to go afterwards

Comment thread gnomad/utils/annotations.py Outdated
@mike-w-wilson mike-w-wilson merged commit 4fbfa73 into main Jul 31, 2025
6 checks passed
@mike-w-wilson mike-w-wilson deleted the mw/merge_hist_diff branch July 31, 2025 15:30
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.

2 participants