Repository: scipy/scipy
Pull request: BUG fixes range of pk variable pre-check
closes #9564 

I applied pre-check for pk range.
previously pk is only checked by summation value (`np.sum(pk) == 1.0`).
In this feature, pk is checked by not only summation value but also each parameter range(0<pk<1).

Please review this PR.

Problem statement:
rv_sample accepts invalid input in scipy.stats
When passing values `(x_k, p_k)` to `stats.rv_discrete`, the only condition that is checked in `rv_sample` is whether `sum(p_k)` is one, but not whether the p_k are between zero and one

### Reproducing code example:

```
from scipy import stats
import numpy as np
custm = stats.rv_discrete(values=((1, 2, 3), (0.5, 1.2, -0.7))) # no error
```

### Scipy/Numpy/Python version information:
```
import sys, scipy, numpy; print(scipy.__version__, numpy.__version__, sys.version_info)
1.1.0 1.15.2 sys.version_info(major=3, minor=5, micro=4, releaselevel='final', serial=0)
```

Hints from issue discussion:
Should I scratch this one?
I will make a code and docs.
@akahard2dj please go for it.

Commit message: pk range check method changes

Patch to review:
diff --git a/scipy/stats/_distn_infrastructure.py b/scipy/stats/_distn_infrastructure.py
index d1b8524c759d..fe3ecdebee32 100644
--- a/scipy/stats/_distn_infrastructure.py
+++ b/scipy/stats/_distn_infrastructure.py
@@ -2578,7 +2578,8 @@ class rv_discrete(rv_generic):
         The tolerance for the generic calculation of moments.
     values : tuple of two array_like, optional
         ``(xk, pk)`` where ``xk`` are integers with non-zero
-        probabilities ``pk``  with ``sum(pk) = 1``.
+        probabilities ``pk``  with ``sum(pk) = 1``. Note: pk should be
+        assigned between 0 to 1.
     inc : integer, optional
         Increment for the support of the distribution.
         Default is 1. (other values have not been tested)
@@ -3362,6 +3363,8 @@ def __init__(self, a=0, b=inf, name=None, badvalue=None,
 
         if len(xk) != len(pk):
             raise ValueError("xk and pk need to have the same length.")
+        if not np.allclose(np.size(np.where((np.array(pk) >= 0.0) & (np.array(pk) <= 1.0))), np.size(pk)):
+            raise ValueError("pk should be assigned between 0 to 1.")
         if not np.allclose(np.sum(pk), 1):
             raise ValueError("The sum of provided pk is not 1.")
 


Code review comments:
diff_hunk: @@ -2578,7 +2578,8 @@ class rv_discrete(rv_generic):
         The tolerance for the generic calculation of moments.
     values : tuple of two array_like, optional
         ``(xk, pk)`` where ``xk`` are integers with non-zero
-        probabilities ``pk``  with ``sum(pk) = 1``.
+        probabilities ``pk``  with ``sum(pk) = 1``. Note: pk should be
path: scipy/stats/_distn_infrastructure.py
text: @user1:
you can continue the sentence 
```
where xk are integers with non-zero ...... and probabilities are floats bounded inside [0., 1.]
```

A "Note" seems weird as it is emphasizing something rather obvious.

@author:
When I hear your opinion, a "Note" seems strange.
So I continue the sentence for description of pk conditions.

diff_hunk: @@ -3362,6 +3363,8 @@ def __init__(self, a=0, b=inf, name=None, badvalue=None,
 
         if len(xk) != len(pk):
             raise ValueError("xk and pk need to have the same length.")
+        if not np.allclose(np.size(np.where((np.array(pk) >= 0.0) & (np.array(pk) <= 1.0))), np.size(pk)):
path: scipy/stats/_distn_infrastructure.py
text: @user1:
This line is too long for PEP8, also you shouldn't compare integers with allclose. You can simply use `!=` and get rid of np.sizes
```python
if np.where((np.array(pk) >= 0.0) & (np.array(pk) <= 1.0))).size != len(pk):
    ...
```


@author:
I modified if conditions. Because of the shape of `np.where()` I did not remove `np.size()`.
I also simplified that line for PEP8.

@user2:
Btw: checking > 0 is enough, sum == 1 takes care of < 1

@user1:
Ah, yes, it then reduces even further to

```
if not all([x >= 0. for x in pk]):
    raise ....
```

@author:
Oh, that would be nice. 
I though about an algorithm too.

BUT this `if condition` is aim to raise an error for range of pk.
To simplify the code, I agree with all. 
In that case, if pk is negative, error is happen.
If pk is greater than 1.0, however, error is not happen (I know that `np.sum(pk)` can catch this error).

How do you think about?

@user1:
Elements of `pk` cannot be negative if we test for `>= 0.` . So first *all elements are negative* condition is established. If elements are greater than `>= 1.` then their sum cannot be `<= 1.`

@author:
ok, I will change error message to

```python
if not all([x >= 0.0 for x in pk]):
    raise ValueError("pk cannot be negative values.")
```

@user2:
numpy.greater_equal might be an option as well

@author:
All requests are reflected.

@user3:
@author should there be a unit test of the `ValueError` now (correctly) thrown? 

@author:
Could you explain more detailed unit test of the `ValueError`?