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

Refactor _assimilate_histogram and _regenerate_histogram #1022

Closed

Conversation

junholee6a
Copy link
Contributor

Issue: #820

The functions _assimilate_histogram and _regenerate_histogram are refactored as directed in the issue. Additionally, _assimilate_histogram no longer rounds bin edges, as this feature doesn't seem necessary and adds an additional argument to _assimilate_histogram.

The methods _regenerate_histogram and _assimilate_histogram hardly use
self and can be moved into the histogram_utils module.
@junholee6a junholee6a changed the title Refactor _assimilate_histogram and _regenerate_histogram Refactor _assimilate_histogram and _regenerate_histogram Aug 22, 2023
@junholee6a
Copy link
Contributor Author

I didn't add unit tests for these functions here because 1) the functions had no direct unit tests in the first place so code coverage won't change, and 2) _assimilate_histogram will be re-written in issue #1017

@junholee6a
Copy link
Contributor Author

Seems like there was a mypy error in the Python 3.8 tests. Will look into that

@taylorfturner
Copy link
Contributor

Seems like there was a mypy error in the Python 3.8 tests. Will look into that

thanks!

auto-merge was automatically disabled September 2, 2023 20:02

Head branch was pushed to by a user without write access

@taylorfturner taylorfturner deleted the branch capitalone:old-old-dev September 26, 2023 15:09
@taylorfturner
Copy link
Contributor

Had to rebuild some branches post 0.10.5 release. Please re-open and change base to dev (i.e. the branch you are merging into) @junholee6a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants