MNT: reference the proper variable in bootstrapper #7868
Conversation
tacaswell
changed the title
NelleV
changed the title
| ii = np.random.randint(M, size=(N, M)) | ||
| bsData = x[ii] | ||
| bs_index = np.random.randint(M, size=(N, M)) | ||
| bsData = data[bs_index] |
There was a problem hiding this comment.
np.random.choice(data, N * len(data), replace=True).reshape((N, M))?
There was a problem hiding this comment.
@anntzer that looks like it would work too. Not sure that performance would be significantly impacted in either direction. So I'm going to punt of figuring out which is better until the refactor associated with MEP 28 like a mentioned in my comment to Nelle.
There was a problem hiding this comment.
np.random.choice(, ...) is basically implemented as array[np.random.randint(...)] so it's the same.
| @@ -1941,8 +1941,8 @@ def _bootstrap_median(data, N=5000): | |||
| M = len(data) | |||
| percentiles = [2.5, 97.5] | |||
|
|
|||
| ii = np.random.randint(M, size=(N, M)) | |||
| bsData = x[ii] | |||
| bs_index = np.random.randint(M, size=(N, M)) | |||
There was a problem hiding this comment.
Much better than before! Using underscored bs_index next to camelcased bsData is a bit strange... bsIndices would be my choice. but I'm nitpicking :)
There was a problem hiding this comment.
The latter is not pep8 compliant, so I'd rather go for the pep8 compliant one though it is not coherent on this particular function.
|
I am very tempted not to wait for appveyor… |
|
Meh, unless we're cutting a release right now I don't see what merging early buys... |
|
@dopplershift We have a lot of pull requests opened right now, and if PRs don't appear on the first page, they tend to be forgotten by reviewers. |
Current coverage is 62.12% (diff: 100%)@@ master #7868 diff @@
==========================================
Files 174 174
Lines 56072 56072
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
Hits 34833 34833
Misses 21239 21239
Partials 0 0
|
|
Thanks @phobson ! |
phobson
changed the title
|
Backported to |
MNT: reference the proper variable in bootstrapper
Cleans up some unclear code that magically worked due to python's scoping rules.
ref comment