Repository: staircase-dev/staircase (language: python)

Issue:
BUG: staircase.make_test_data fails with numpy < 1.17
- [ ] I have checked that this issue has not already been reported.

- [ ] I have confirmed this bug exists on the latest version of staircase.

---

**Note**: Please read [this guide](https://matthewrocklin.com/blog/work/2018/02/28/minimal-bug-reports) detailing how to provide the necessary information for us to reproduce your bug.

#### Code Sample, a copy-pastable example

```python
>>> import staircase as sc
>>> sc.make_test_data()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\rclement\Documents\temp11\.venv\lib\site-packages\staircase\test_data.py", line 56, in make_test_data
    return _make_test_data(dates=dates, positive_only=positive_only, seed=seed)
  File "C:\Users\rclement\Documents\temp11\.venv\lib\site-packages\staircase\test_data.py", line 71, in _make_test_data
    rng = np.random.default_rng()
AttributeError: module 'numpy.random' has no attribute 'default_rng'

```

#### Problem description

#### Expected Output

#### Dependency Versions``

<details>

staircase 2.0.0
pandas 1.0.0
numpy 1.16.0

</details>



Discussion / hints:
I would consider raising the numpy minimum version requirements to 1.19 for python 3.6 onwards.

[numpy news](https://numpy.org/news/)
`Jun 20, 2020 – NumPy 1.19.0 is now available. This is the first release without Python 2 support, hence it was a “clean-up release”. The minimum supported Python version is now Python 3.6. An important new feature is that the random number generation infrastructure that was introduced in NumPy 1.17.0 is now accessible from Cython.`

Hi Pablo, thanks for taking an interest in staircase!

Support for Python 3.6 will be dropped in the next release of staircase anyway, but I think its worth discussing a couple of things.

Python 3.6 was released late 2016, when numpy 1.11.2 was the latest release, and numpy 1.19 was released 3.5 years after this.  Because staircase is a library we want the dependencies to be as accommodating as possible.  This is made even more important given the complete mess that dependency resolution and packaging in Python is (and continues to be).  

I agree that it in general it would be preferable for users to use later versions of numpy, for many reasons, but I don't think it is our place to enforce that on them if the staircase code can work with earlier versions.  It seems the only instance of an earlier numpy version causing a problem for us is this `make_test_data` function, which is not exactly a huge problem, and could be solved with a workaround which tests for numpy version before executing code.

Back to dependency resolution... let's say we had given numpy a lower bound of 1.19 from the start.  It could be possible that a user has another package in their environment which requires numpy < 1.19, and the environment would not be resolvable if including staircase. Or perhaps there is a user who has a numpy version < 1.19 and they wish to introduce staircase into their environment without changing any of their existing dependencies - they would not be able to and there doesn't seem to be a good reason to prevent them from doing so since the code should still work.

Since published versions of staircase exist which are compatible with numpy < 1.19, publishing a new version which restricts numpy >= 1.19 will result in two cases for environment resolution:

i) The user has no constraints in their environment which force numpy < 1.19 and their dependency resolver (eg pip, poetry, conda etc) will generally choose the latest version of numpy it can (which will be >= 1.19 but perhaps not the very latest depending on other constraints).  In this case the dependency resolver is free to choose the newly published staircase package where numpy is required to be above 1.19

ii) The user has a constraint that numpy < 1.19 - either directly introduced by themselves, or by a dependency (or a dependency of a dependency etc).  In this case the dependency resolver will just choose an earlier version of staircase that is compatible with the earlier numpy version.  The user will have been prevented from using the later version of staircase (again, for no concrete reason).

It's hard to see what benefit introducing the numpy 1.19 restriction would have, but happy to keep the conversation going.  I'm generally flying solo on this project so it's good to have other opinions.

Thanks for the explanation, I'm aware of the pros and cons and normally I would still say that the inconveniences are greater than the benefits, but as you stated:

_It seems the only instance of an earlier numpy version causing a problem for us is this make_test_data function, which is not exactly a huge problem_

So I agree with you that being only one case maybe the best idea is just to patch it. [some link for the possible implementation for consulting in the near future](https://stackoverflow.com/questions/62077194/can%C2%B4t-import-qiskit-attribute-error-in-numpy-numpy-random-has-no-attribute) 

fantastic :)

Fix patch:
diff --git a/docs/release_notes/changelog.rst b/docs/release_notes/changelog.rst
index dc0c76e..769195f 100644
--- a/docs/release_notes/changelog.rst
+++ b/docs/release_notes/changelog.rst
@@ -9,6 +9,7 @@ UNRELEASED
 
 
 - dropped support for Python 3.6
+- bugfix for :func:`staircase.make_test_data` which failed with numpy < 1.17 (#GH80)
 
 Please list new changes above this comment
 


Test patch:
diff --git a/staircase/test_data.py b/staircase/test_data.py
index a831cf2..7ab171c 100644
--- a/staircase/test_data.py
+++ b/staircase/test_data.py
@@ -66,21 +66,30 @@ def make_test_data(dates=True, positive_only=True, groups=(), seed=None):
     return pd.concat(dfs)
 
 
+def _get_random_functions(seed=None):
+    major, minor, *_ = np.__version__.split(".")
+
+    if f"{major}.{minor}" in ("1.14", "1.15", "1.16"):
+        rs = np.random.RandomState(seed)
+        return rs.randint, rs.uniform
+
+    rng = np.random.default_rng(seed)
+    return rng.integers, rng.random
+
+
 def _make_test_data(dates=True, positive_only=True, seed=None):
-    if seed is None:
-        rng = np.random.default_rng()
-    else:
-        rng = np.random.default_rng(seed)
 
-    n_intervals = rng.integers(100, 1000)
-    smallest_interval = rng.integers(1, 6)
-    largest_interval = rng.integers(15, 20)
+    random_integers, random_floats = _get_random_functions(seed)
+
+    n_intervals = random_integers(100, 1000)
+    smallest_interval = random_integers(1, 6)
+    largest_interval = random_integers(15, 20)
     interval_lengths = (
-        rng.random(size=n_intervals) * (largest_interval - smallest_interval)
+        random_floats(size=n_intervals) * (largest_interval - smallest_interval)
         + smallest_interval
     )
 
-    start = rng.random(size=n_intervals) * 120 - 20
+    start = random_floats(size=n_intervals) * 120 - 20
     end = start + interval_lengths
 
     if dates:
@@ -91,11 +100,17 @@ def _make_test_data(dates=True, positive_only=True, seed=None):
             end * 365 / 100 * pd.Timedelta(1, "day")
         )
 
-    value = rng.choice(range(-5, 6), n_intervals)
+    value = random_integers(-5, 6, n_intervals)
     if positive_only:
         value = value + 6
 
-    data = pd.DataFrame({"start": start, "end": end, "value": value,})
+    data = pd.DataFrame(
+        {
+            "start": start,
+            "end": end,
+            "value": value,
+        }
+    )
 
     if dates:
         data["start"] = data["start"].dt.floor("min")