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

Issue:
ENH: Add Stairs.from_values method
#### Is your feature request related to a problem?

Allow a user who has timeseries data in pandas, which represents step function values, to create a Stairs instance from this data.

#### Describe the solution you'd like

Similar to 
https://github.com/staircase-dev/staircase/blob/00da1209eec21d3ea11f0f18f538637d2bae5ecf/staircase/core/stairs.py#L73

Parameters should include
- initial_value (float or nan)
- values (pandas.Series)
- closed (either "left" or "right")

values should be indexed by change points of step function.  The values of *values* should be step function values when approaching the change points from the right (this is the internal representation used in "value" column of Stairs._data).  

#### Additional context

See https://www.staircase.dev/en/latest/development/guide/internals.html for more context


Discussion / hints:
The pandas Series should be monotonic? For example, does the following Series makes sense: `s = pd.Series([1, 2, 3, 4, 5], index=[-1, 1, -3, 4, 2])`? Notice that the change points (the index) go back and forth.
I think so, ultimately it needs to be stored with a monotonic index internally in the Stairs object so I think it's reasonable to force the user to have it sorted.  Was that your intuition with fresh eyes on the project?

We should also enforce that the values of the Series are numeric.  As for the index it's probably going to be numeric, pandas.Timestamp, pandas.Timedelta, numpy.datetime64, numpy.timedelta64, datetime.datetime or datetime.timedelta.  I think we want to check that the index is one of these types, and warn if it is not.  The pandas.api.types module has many functions which can do these checks.  Warning instead of an exception would be better because theoretically any class which can be totally ordered should work with staircase, but I'm wanting to prevent the possibility of someone trying to pass a Series which has a index of mixed types, which will definitely cause Staircase to fail.

We could probably raise an exception if the index contains np.inf: we definitely can't have that.
And we should convert any np.inf in the values of the Series, to np.nan and warn the user, that this is happening.
> I think so, ultimately it needs to be stored with a monotonic index internally in the Stairs object so I think it's reasonable to force the user to have it sorted. Was that your intuition with fresh eyes on the project?
Yes, I believe that both the index and values should be monotonic.

> We could probably raise an exception if the index contains np.inf: we definitely can't have that.

Agreed

> And we should convert any np.inf in the values of the Series, to np.nan and warn the user, that this is happening. 

I see no issue with this

Could you point to some unit test I could use as a guide? I see a few in [here](https://github.com/staircase-dev/staircase/blob/master/tests/test_floats/test_floats_construction.py) but I don't know if any of the fixtures there can be used to test this method. 



Fix patch:
diff --git a/.gitignore b/.gitignore
index 64713f5..559d6ef 100644
--- a/.gitignore
+++ b/.gitignore
@@ -138,3 +138,4 @@ READMEold.md
 docs - copy/
 tests/mtest_stairs_module_dates.py
 tests/mtest_stairs_module.py
+.idea
diff --git a/docs/reference/Stairs.rst b/docs/reference/Stairs.rst
index 17a1b4d..99eb8af 100644
--- a/docs/reference/Stairs.rst
+++ b/docs/reference/Stairs.rst
@@ -12,6 +12,7 @@ Constructor & basic methods
 
    Stairs.__init__
    Stairs.copy
+   Stairs.from_values
    Stairs.sample
    Stairs.limit
    Stairs.layer
@@ -19,9 +20,9 @@ Constructor & basic methods
    Stairs.step_values
    Stairs.step_points
    Stairs.to_frame
-   
+
 .. _api.arithmetic_operators:
- 
+
 Arithmetic operators
 ~~~~~~~~~~~~~~~~~~~~
 
@@ -39,7 +40,7 @@ Arithmetic operators
    Stairs.rdivide
 
 .. _api.relational_operators:
-   
+
 Relational operators
 ~~~~~~~~~~~~~~~~~~~~
 
@@ -53,7 +54,7 @@ Relational operators
    Stairs.eq
    Stairs.ne
    Stairs.identical
-   
+
 .. _api.logical_operators:
 
 Logical operators
@@ -61,7 +62,7 @@ Logical operators
 
 .. autosummary::
    :toctree: api/
-   
+
    Stairs.make_boolean
    Stairs.invert
    Stairs.logical_and
@@ -78,7 +79,7 @@ Statistical operators
 
 .. autosummary::
    :toctree: api/
-   
+
    Stairs.cov
    Stairs.corr
 
@@ -108,23 +109,23 @@ Summary statistics
    Stairs.quantiles
    Stairs.ecdf
    Stairs.hist
-   
+
 Plotting
-~~~~~~~~~~~~~~~~~~ 
+~~~~~~~~~~~~~~~~~~
 
 .. autosummary::
    :toctree: api/
    :template: autosummary/accessor_callable.rst
 
    Stairs.plot
-   
-   
+
+
 Miscellaneous functions
 ~~~~~~~~~~~~~~~~~~~~~~~
 
 .. autosummary::
    :toctree: api/
-   
+
    Stairs.clip
    Stairs.mask
    Stairs.where
diff --git a/docs/release_notes/changelog.rst b/docs/release_notes/changelog.rst
index 8857723..896b81a 100644
--- a/docs/release_notes/changelog.rst
+++ b/docs/release_notes/changelog.rst
@@ -7,6 +7,7 @@ Changelog
 
 UNRELEASED
 
+- added :meth:`staircase.Stairs.from_values` (#GH88)
 - bugfix for array aggregation where array contains a constant value step function over non-float domain (#GH119)
 
 Please list new changes above this comment
@@ -61,7 +62,7 @@ Contributors: @amagee (Andrew Magee)
 
 - bugfix for missing *lower_how* and *upper_how* parameters in :meth:`staircase.Stairs.resample`
 - renamed Stairs._values to avoid error when performing Series.groupby.sum in pandas>=1.2
- 
+
 
 **v1.6.2 2020-01-13**
 
diff --git a/staircase/core/stairs.py b/staircase/core/stairs.py
index ff81780..583484b 100644
--- a/staircase/core/stairs.py
+++ b/staircase/core/stairs.py
@@ -7,9 +7,16 @@
 modelling step functions. See :ref:`Getting Started <getting_started>` for more information.
 """
 
+import warnings
 
 import numpy as np
 import pandas as pd
+from pandas.api.types import (
+    is_datetime64_dtype,
+    is_number,
+    is_numeric_dtype,
+    is_timedelta64_dtype,
+)
 
 from staircase import docstrings
 from staircase.constants import inf
@@ -78,6 +85,57 @@ def _new(cls, initial_value, data, closed="left"):
         new_instance._valid_values = False if data is None else "value" in data.columns
         return new_instance
 
+    @classmethod
+    def from_values(cls, initial_value, values=None, closed="left"):
+        """
+        Construct :class:`Stairs` from :class:`pandas.Series`.
+
+        Parameters
+        ----------
+        initial_value : float, default 0
+            The value of the step function at negative infinity.
+        values : :class:`pandas.Series`
+            The step function values' when approaching the change points from the right
+        closed : {"left", "right"}
+            Indicates whether the half-open intervals comprising the step function should be interpreted
+            as left-closed or right-closed.
+
+        Returns
+        -------
+        :class:`Stairs`
+        """
+
+        if not isinstance(values, pd.Series) or values.empty:
+            raise ValueError("values must be a not empty Series")
+
+        if not (
+            is_numeric_dtype(values.index)
+            or is_datetime64_dtype(values.index)
+            or is_timedelta64_dtype(values.index)
+        ):
+            warnings.warn("The index of data is not numeric, or time based")
+
+        if np.isinf(values.index).any():
+            raise ValueError("Invalid value for Series index")
+
+        if not is_numeric_dtype(values) or not is_number(initial_value):
+            raise ValueError("Invalid dtype for from_values()")
+
+        if not values.index.is_monotonic_increasing:
+            raise ValueError("Series index must be monotonic")
+
+        series_values_inf_mask = np.isinf(values)
+        if series_values_inf_mask.any():
+            values = values.replace([np.inf], np.nan)
+            warnings.warn("Infinity values detected and have been converted to NaN")
+
+        new_instance = cls(closed=closed)
+        new_instance.initial_value = initial_value
+        new_instance._data = values.to_frame("value")
+        new_instance._valid_deltas = False
+        new_instance._valid_values = True
+        return new_instance
+
     def _has_na(self):
         return np.isnan(self._data.values).any() or np.isnan(self.initial_value)
 


Test patch:
diff --git a/tests/test_floats/test_floats_construction.py b/tests/test_floats/test_floats_construction.py
index fce1b3b..aa039cf 100644
--- a/tests/test_floats/test_floats_construction.py
+++ b/tests/test_floats/test_floats_construction.py
@@ -332,6 +332,44 @@ def test_layering_frame(s1_fix):
     assert Stairs(df, "start", "end", "value").identical(s1_fix)
 
 
+@pytest.mark.parametrize(
+    "closed",
+    ["left", "right"],
+)
+@pytest.mark.parametrize(
+    "initial_value",
+    [0, -1, 1],
+)
+def test_from_values(initial_value, closed):
+    # this corresponds to the step function produced by S1 method
+    values = pd.Series([-1.75, 0.25, 2.75, 2.00, -0.5, 0], index=[-4, 1, 3, 5, 6, 10])
+    sf = Stairs.from_values(
+        initial_value=initial_value,
+        values=values + initial_value,
+        closed=closed,
+    )
+    assert sf.identical(s1(closed) + initial_value)
+
+
+@pytest.mark.parametrize(
+    "index, values",
+    [
+        ([-np.inf], [0]),
+        ([np.inf], [0]),
+        ([1, 0], [10, 20]),
+        ([0], ["1"]),
+        ([], []),
+    ],
+)
+def test_from_values_exception(index, values):
+    with pytest.raises(ValueError):
+        Stairs.from_values(
+            initial_value=0,
+            values=pd.Series(values, index=index),
+            closed="left",
+        )
+
+
 def test_layering_trivial_1(s1_fix):
     assert s1_fix.copy().layer(1, 1).identical(s1_fix)