-
Notifications
You must be signed in to change notification settings - Fork 46
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
Region-k-means seed #294
Region-k-means seed #294
Conversation
Codecov Report
@@ Coverage Diff @@
## main #294 +/- ##
=======================================
+ Coverage 72.9% 73.2% +0.3%
=======================================
Files 23 23
Lines 2402 2402
Branches 460 459 -1
=======================================
+ Hits 1750 1758 +8
+ Misses 584 580 -4
+ Partials 68 64 -4
|
@sjsrey @knaaptime gently pining for review (when time permits). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The seed stuff looks good. Just one question related to RegionMixin that I left.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting the lack of use of RegionMixin
Resolves #213.
After more digging I found that we were not providing the functionality of actually passing in a seed value to the
base._seeds()
function. Therefore, the resultantnumpy.random.choice()
function within could never be reproduced and was screwing tests up.This PR also:
base.py
(xref fill and and standardize docstrings #173, short-to-mid term plan for improving docs, structure, and formatting #280)RegionMixin()