ENH: Add closed generator to randint #15
Conversation
Add closed option to randint to simplify some cases
|
Can we hold off on this until after we first merge randomgen to numpy? I would hate for the merge to get bogged down in discussions of the merit of this enhancement. Or was there consensus around adding this kwarg to randint? |
|
Im happy to wait.
…On Tue, Apr 16, 2019, 08:19 Matti Picus ***@***.***> wrote:
Can we hold off on this until after we first merge randomgen to numpy? I
would hate for the merge to get bogged down in discussions of the merit of
this enhancement. Or was there consensus around adding this kwarg to
randint?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#15 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AFU5RfjkNylOvGrord_II2B7SiSXpgQpks5vhXmTgaJpZM4cxgPC>
.
|
|
xref numpy/numpy#13164
On Tue, Apr 16, 2019, 08:20 Kevin Sheppard <[email protected]>
wrote:
… Im happy to wait.
On Tue, Apr 16, 2019, 08:19 Matti Picus ***@***.***> wrote:
> Can we hold off on this until after we first merge randomgen to numpy? I
> would hate for the merge to get bogged down in discussions of the merit of
> this enhancement. Or was there consensus around adding this kwarg to
> randint?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#15 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AFU5RfjkNylOvGrord_II2B7SiSXpgQpks5vhXmTgaJpZM4cxgPC>
> .
>
|
numpy/random/bounded_integers.pyx.in
Outdated
| raise ValueError('high is out of bounds for {{nptype}}') | ||
| if np.any(np.greater_equal(low_arr, high_arr)): | ||
| if np.any(low_high_comp(low_arr, high_arr)): | ||
| raise ValueError('low >= high') |
There was a problem hiding this comment.
This error message doesn't match the comparator
There was a problem hiding this comment.
I think that string probably needs to be dynamic and change depending on whether closed or not.
There was a problem hiding this comment.
low=high is ok for closed, but only low<high for open.
Use correct error message when usign a closed interval
|
I pushed some small changes in response to the feedback above. This is needed since the closed interval is in the API through |
|
In light of the discussion, I would like to put this in now. Any objections? |
|
It failed on my Travis, FWIW. It is failing due to changes in pytest. |
numpy#13550 should fix that |
Merge in numpy-hpy from ss/array_array3 to labs-hpy-port * commit 'a197852c5099cec81ed8cdfde0a59a29316db2a3': hpy_raw_array_assign_scalar HPyArray_AssignRawScalar and HPyArray_EquivTypes
3 participants
Add closed option to randint to simplify some cases