Skip to content

This is PR #2727, but rebased - #2793

Merged
certik merged 5 commits into
numpy:masterfrom
certik:pull-request-2727
Dec 13, 2012
Merged

certik merged 5 commits into
numpy:masterfrom
certik:pull-request-2727

Conversation

@certik

@certik certik commented Dec 6, 2012

Copy link
Copy Markdown
Contributor

Also I regenerated the cython file, just in case. We need to run tests on this one to make sure that all passes.

The size argument to random.choice should work like it does for all
other functions in random as well.
Random choice used np.unique to find new indices when replace
was False and p given. This is wrong since unique will sort the
indices. This solves the bug, but likely not ideal.
Thanks to @alan-isaac for pointing out the 0-d vs. scalar issue.
@certik

certik commented Dec 13, 2012

Copy link
Copy Markdown
Contributor Author

Hm, Travis didn't run tests.... I pushed in a "Travis ping" commit. Here we go.

@certik

certik commented Dec 13, 2012

Copy link
Copy Markdown
Contributor Author

Ok, all tests passed, so I am removing the "ping" commit and merging.

certik added a commit that referenced this pull request Dec 13, 2012
@certik
certik merged commit cdeb48b into numpy:master Dec 13, 2012
@seberg

seberg commented Dec 13, 2012

Copy link
Copy Markdown
Member

@certik it was intentional to push this to master and not 1.7.? I just wanted to double check that the results look reasonable and wondered about them being nonsense before realizing that this went into master.

@certik

certik commented Dec 14, 2012

Copy link
Copy Markdown
Contributor Author

All bug fixes go into master first. I just rebased your original PR, which was also against master.

Should this be backported to 1.7 as well?

@seberg

seberg commented Dec 14, 2012

Copy link
Copy Markdown
Member

Ah, thanks... I would think it should be in 1.7., because it changes
what is returned by it and fixes a bug and the function is new in 1.7.

@certik certik mentioned this pull request Dec 14, 2012
@certik

certik commented Dec 14, 2012

Copy link
Copy Markdown
Contributor Author

Ok. Backported in #2820.

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