Repository: casact/chainladder-python

Issue:
How to correctly apply drop_valuation to quarterly triangles?
### Discussed in https://github.com/casact/chainladder-python/discussions/455

<div type='discussions-op-text'>

<sup>Originally posted by **contenrico** September  7, 2023</sup>
Hello,

I'm working with a quarterly triangle and I'm trying to exclude the latest diagonal (valuation date 2022-12). This is the code I'm running:

`cl.Development(drop_valuation='2022').fit_transform(incurred).link_ratio`

However, the excluded link ratios are the following:

![image](https://github.com/casact/chainladder-python/assets/137546866/681a2528-fda7-485e-bd3e-8b1000bea142)

This is quite odd, as I would expect the following points to be excluded:
(2022, 3), (2022, 6), (2022, 9)
(2021, 15), (2021, 18), (2021, 21)
(2020, 27), (2020, 30), (2020, 33)
etc.

Is there a way to use drop_valuation that achieves this?

Thanks!</div>

Hints from issue comments:
@henrydingliu: can you create a reproducible example using the sample data available in the package?@contenrico: Sure:

`triangle = cl.load_sample('quarterly')`
`incurred = triangle['incurred']`
`cl.Development(drop_valuation='2005').fit_transform(incurred).link_ratio`@kennethshsu: Hi @contenrico, did you ever resolve your issue?

My understanding is that you are just trying to drop the latest diagonal of your triangle, is that correct? There's no need to use `cl.Development()` for this. You can do this:

```python
import chainladder as cl
triangle = cl.load_sample('quarterly')
incurred = triangle['incurred']
incurred[incurred.valuation < '2006-1-1']
```

However, if you need the link ratios, excluding information from the latest diagonal, you can then do this:
```python
cl.Development().fit(incurred[incurred.valuation < '2006-1-1']).ldf_
```

Does this help?@contenrico: Hi @kennethshsu,

I was indeed trying to drop the latest diagonal from my triangle, but for the specific purpose of calculating link ratios - so I think I do need to use `cl.Development()`.

Your second code snippet does exactly that, so I think that works! I guess the only improvement I need is to make that `'2006-1-1'` dynamic based on the valuation date, so I can always exclude the latest diagonal (or latest two, three, etc.).

Thank you!@kennethshsu: If you need to access `2006-1-1` as a `TimeStamp` variable, you can call `.valuation_date` on your `Triangle` object. Continuing the example:
```python
incurred.valuation_date
```

Then, you can calculate the ldfs or cdfs as usual:
```python
cl.Development().fit(incurred[incurred.valuation < incurred.valuation_date]).cdf_
```

With this, you'll be able to manipulate `incurred.valuation_date`, for example, excluding the latest 6 months, with
```python
from dateutil.relativedelta import relativedelta
date_x6months = incurred.valuation_date - relativedelta(months=6)
cl.Development().fit(incurred[incurred.valuation < date_x6months]).cdf_
```

Now, with all that said, I don't know why `drop_valuation` in `cl.Development` isn't working as expected. I can try to look into this.@kennethshsu: Tests:

```python
# Annual development work 
raa = cl.load_sample("raa")
assert cl.Development(drop_valuation='1981-12-31').fit_transform(raa).cdf_ != cl.Development(drop_valuation='1982-12-31').fit_transform(raa).cdf_
assert cl.Development(drop_valuation='1982-12-31').fit_transform(raa).cdf_ != cl.Development(drop_valuation='1983-12-31').fit_transform(raa).cdf_
assert cl.Development(drop_valuation='1981-12-31').fit_transform(raa).cdf_ != cl.Development(drop_valuation='1983-12-31').fit_transform(raa).cdf_

# but non-annual don't
assert cl.Development().fit_transform(incurred).cdf_ != cl.Development(drop_valuation='1995-03-31').fit_transform(incurred).cdf_
assert cl.Development().fit_transform(incurred).cdf_ != cl.Development(drop_valuation='1995-06-30').fit_transform(incurred).cdf_
assert cl.Development(drop_valuation='1995-03-31').fit_transform(incurred).cdf_ != cl.Development(drop_valuation='1995-06-30').fit_transform(incurred).cdf_ #this fails now
assert cl.Development(drop_valuation='1995-06-30').fit_transform(incurred).cdf_ != cl.Development(drop_valuation='1995-09-30').fit_transform(incurred).cdf_ #this fails now


Solution patch:
diff --git a/chainladder/core/triangle.py b/chainladder/core/triangle.py
index ededf2b..7910583 100644
--- a/chainladder/core/triangle.py
+++ b/chainladder/core/triangle.py
@@ -19,16 +19,10 @@ try:
 except ImportError:
     db = None
 
-from typing import (
-    Optional,
-    TYPE_CHECKING
-)
+from typing import Optional, TYPE_CHECKING
 
 if TYPE_CHECKING:
-    from pandas import (
-        DataFrame,
-        Series
-    )
+    from pandas import DataFrame, Series
     from numpy.typing import ArrayLike
     from pandas._libs.tslibs.timestamps import Timestamp  # noqa
     from pandas.core.interchange.dataframe_protocol import DataFrame as DataFrameXchg
@@ -138,7 +132,7 @@ class Triangle(TriangleBase):
         pattern=False,
         trailing: bool = True,
         *args,
-        **kwargs
+        **kwargs,
     ):
 
         # If data are present, validate the dimensions.
@@ -151,7 +145,7 @@ class Triangle(TriangleBase):
             index=index,
             columns=columns,
             origin=origin,
-            development=development
+            development=development,
         )
 
         # Store dimension metadata.
@@ -164,43 +158,33 @@ class Triangle(TriangleBase):
             index=index,
             columns=columns,
             origin=origin,
-            development=development
+            development=development,
         )
         # Conform origins and developments to datetimes and determine the lowest grains.
         origin_date: Series = self._to_datetime(
-            data=data,
-            fields=origin,
-            date_format=origin_format
-        ).rename(
-            "__origin__"
-        )
+            data=data, fields=origin, date_format=origin_format
+        ).rename("__origin__")
 
         self.origin_grain: str = self._get_grain(
-            dates=origin_date,
-            trailing=trailing,
-            kind="origin"
+            dates=origin_date, trailing=trailing, kind="origin"
         )
 
         development_date = self._set_development(
             data=data,
             development=development,
             development_format=development_format,
-            origin_date=origin_date
+            origin_date=origin_date,
         )
 
         self.development_grain = self._get_grain(
-            dates=development_date,
-            trailing=trailing,
-            kind="development"
+            dates=development_date, trailing=trailing, kind="development"
         )
 
         # Ensure that origin_date values represent the beginning of the period.
         # i.e., 1990 means the start of 1990.
         origin_date: Series = origin_date.dt.to_period(
             self.origin_grain
-        ).dt.to_timestamp(
-            how="s"
-        )
+        ).dt.to_timestamp(how="s")
 
         # Ensure that development_date values represent the end of the period.
         # i.e., 1990 means the end of 1990 assuming annual development periods.
@@ -214,7 +198,7 @@ class Triangle(TriangleBase):
             origin_date=origin_date,
             development_date=development_date,
             index=index,
-            columns=columns
+            columns=columns,
         )
 
         # Fill in missing periods with zeros.
@@ -307,7 +291,7 @@ class Triangle(TriangleBase):
             key_idx=key_idx,
             columns=columns,
             orig_idx=orig_idx,
-            dev_idx=dev_idx
+            dev_idx=dev_idx,
         )
 
         # Construct Sparse multidimensional array.
@@ -351,11 +335,7 @@ class Triangle(TriangleBase):
 
     @staticmethod
     def _split_ult(
-            data: DataFrame,
-            index: list,
-            columns: list,
-            origin: list,
-            development: list
+        data: DataFrame, index: list, columns: list, origin: list, development: list
     ) -> tuple[DataFrame, Triangle]:
         """Deal with triangles with ultimate values"""
         ult = None
@@ -365,6 +345,7 @@ class Triangle(TriangleBase):
             and data[development[0]].dtype == "<M8[ns]"
         ):
             u = data[data[development[0]] == options.ULT_VAL].copy()
+            print("332")
             if len(u) > 0 and len(u) != len(data):
                 ult = Triangle(
                     u,
@@ -375,6 +356,7 @@ class Triangle(TriangleBase):
                 )
                 ult.ddims = pd.DatetimeIndex([options.ULT_VAL])
                 data = data[data[development[0]] != options.ULT_VAL]
+            print("343")
         return data, ult
 
     @property
@@ -841,7 +823,7 @@ class Triangle(TriangleBase):
         start=None,
         end=None,
         ultimate_lag=None,
-        **kwargs
+        **kwargs,
     ):
         """Allows for the trending of a Triangle object along either a valuation
         or origin axis.  This method trends using days and assumes a years is
diff --git a/chainladder/development/base.py b/chainladder/development/base.py
index a1a2107..1637d86 100644
--- a/chainladder/development/base.py
+++ b/chainladder/development/base.py
@@ -86,7 +86,6 @@ class DevelopmentBase(BaseEstimator, TransformerMixin, EstimatorIO, Common):
 
     def _drop_adjustment(self, X, link_ratio):
         weight = X.nan_triangle[:, :-1]
-
         if self.drop is not None:
             weight = weight * self._drop(X)
 
@@ -237,6 +236,7 @@ class DevelopmentBase(BaseEstimator, TransformerMixin, EstimatorIO, Common):
 
     # for drop_above and drop_below
     def _drop_x(self, drop_above, drop_below, X, link_ratio, preserve):
+
         # this is safe because each triangle by index and column has
         link_ratios_len = link_ratio.shape[3]
 
@@ -312,23 +312,21 @@ class DevelopmentBase(BaseEstimator, TransformerMixin, EstimatorIO, Common):
 
     def _drop_valuation(self, X):
         xp = X.get_array_module()
+
         if type(self.drop_valuation) is not list:
             drop_valuation = [self.drop_valuation]
         else:
             drop_valuation = self.drop_valuation
-        v = pd.PeriodIndex(drop_valuation, freq=X.origin_grain).to_timestamp(how="e")
-        arr = 1 - xp.nan_to_num(X[X.valuation.isin(v)].values[0, 0] * 0 + 1)
-        ofill = X.shape[-2] - arr.shape[-2]
-        dfill = X.shape[-1] - arr.shape[-1]
-        if ofill > 0:
-            arr = xp.concatenate(
-                (arr, xp.repeat(xp.ones(arr.shape[-1])[None], ofill, 0)), 0
-            )
-        if dfill > 0:
-            arr = xp.concatenate(
-                (arr, xp.repeat(xp.ones(arr.shape[-2])[..., None], dfill, -1)), -1
-            )
-        return arr[:, :-1]
+
+        drop_valuation_vector = pd.PeriodIndex(
+            drop_valuation, freq=X.development_grain
+        ).to_timestamp(how="e")
+
+        tri_w = X * 0 + 1
+        tri_w = tri_w[~tri_w.valuation.isin(drop_valuation_vector)]
+        tri_w = xp.nan_to_num(tri_w.values[0, 0])
+
+        return tri_w[:, :-1]
 
     def _drop(self, X):
         xp = X.get_array_module()
@@ -370,8 +368,10 @@ class DevelopmentBase(BaseEstimator, TransformerMixin, EstimatorIO, Common):
 
         if (self.drop_high is not None) | (self.drop_low is not None):
             w = w * self._drop_n_func(factor * num_to_nan(w), secondary_rank)
+
         w_tri = factor.copy()
         w_tri.values = num_to_nan(w)
+
         return w_tri
 
     def _assign_n_periods_weight_func(self, factor):
@@ -436,6 +436,7 @@ class DevelopmentBase(BaseEstimator, TransformerMixin, EstimatorIO, Common):
         return xp.nan_to_num(arr)[None, None]
 
     def _drop_valuation_func(self, factor):
+
         # get the appropriate backend for nan_to_num
         xp = factor.get_array_module()
         # turn single drop_valuation parameter to list if necessary
diff --git a/chainladder/development/development.py b/chainladder/development/development.py
index e7b18a7..893d84a 100644
--- a/chainladder/development/development.py
+++ b/chainladder/development/development.py
@@ -156,6 +156,7 @@ class Development(DevelopmentBase):
             obj, n_periods_
         ) * self._drop_adjustment(obj, link_ratio)
         w = num_to_nan(self.w_ / (x ** (exponent)))
+
         params = WeightedRegression(axis=2, thru_orig=True, xp=xp).fit(x, y, w)
 
         if self.n_periods != 1:
@@ -175,9 +176,11 @@ class Development(DevelopmentBase):
 
         params = xp.concatenate((params.slope_, params.sigma_, params.std_err_), 3)
         params = xp.swapaxes(params, 2, 3)
+
         self.ldf_ = self._param_property(obj, params, 0)
         self.sigma_ = self._param_property(obj, params, 1)
         self.std_err_ = self._param_property(obj, params, 2)
+
         resid = -obj.iloc[..., :-1] * self.ldf_.values + obj.iloc[..., 1:].values
         std = xp.sqrt((1 / num_to_nan(w)) * (self.sigma_**2).values)
         resid = resid / num_to_nan(std)
diff --git a/docs/getting_started/online_sandbox/sandbox_workbook_blank.ipynb b/docs/getting_started/online_sandbox/sandbox_workbook_blank.ipynb
index c63c08b..746f39e 100644
--- a/docs/getting_started/online_sandbox/sandbox_workbook_blank.ipynb
+++ b/docs/getting_started/online_sandbox/sandbox_workbook_blank.ipynb
@@ -838,7 +838,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.9.17"
+   "version": "3.9.19"
   }
  },
  "nbformat": 4,
diff --git a/requirements.txt b/requirements.txt
index b6f6cb7..59030b5 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -4,4 +4,4 @@ numba>0.54
 sparse>=0.9
 matplotlib
 dill
-patsy
+patsy
\ No newline at end of file


Test patch:
diff --git a/chainladder/development/tests/test_development.py b/chainladder/development/tests/test_development.py
index a5ef179..54086cc 100644
--- a/chainladder/development/tests/test_development.py
+++ b/chainladder/development/tests/test_development.py
@@ -174,6 +174,52 @@ def test_dropabovebelow():
     assert np.all(lhs == rhs)
 
 
+def test_drop_valuation():
+    raa = cl.load_sample("raa")
+    assert (
+        cl.Development(drop_valuation="1981-12-31").fit_transform(raa).cdf_
+        != cl.Development(drop_valuation="1982-12-31").fit_transform(raa).cdf_
+    )
+    assert (
+        cl.Development(drop_valuation="1982-12-31").fit_transform(raa).cdf_
+        != cl.Development(drop_valuation="1983-12-31").fit_transform(raa).cdf_
+    )
+    assert (
+        cl.Development(drop_valuation="1981-12-31").fit_transform(raa).cdf_
+        != cl.Development(drop_valuation="1983-12-31").fit_transform(raa).cdf_
+    )
+
+    quarterly = cl.load_sample("quarterly")
+    assert (
+        cl.Development().fit_transform(quarterly["incurred"]).cdf_
+        != cl.Development(drop_valuation="1995-03-31")
+        .fit_transform(quarterly["incurred"])
+        .cdf_
+    )
+    assert (
+        cl.Development().fit_transform(quarterly["incurred"]).cdf_
+        != cl.Development(drop_valuation="1995-06-30")
+        .fit_transform(quarterly["incurred"])
+        .cdf_
+    )
+    assert (
+        cl.Development(drop_valuation="1995-03-31")
+        .fit_transform(quarterly["incurred"])
+        .cdf_
+        != cl.Development(drop_valuation="1995-06-30")
+        .fit_transform(quarterly["incurred"])
+        .cdf_
+    )
+    assert (
+        cl.Development(drop_valuation="1995-06-30")
+        .fit_transform(quarterly["incurred"])
+        .cdf_
+        != cl.Development(drop_valuation="1995-09-30")
+        .fit_transform(quarterly["incurred"])
+        .cdf_
+    )
+
+
 def test_assymetric_development(atol):
     quarterly = cl.load_sample("quarterly")["paid"]
     xp = np if quarterly.array_backend == "sparse" else quarterly.get_array_module()
@@ -254,9 +300,7 @@ def test_new_drop_5a(clrd):
 def test_new_drop_6(clrd):
     clrd = clrd.groupby("LOB")[["IncurLoss", "CumPaidLoss"]].sum()
     # drop_above/below without preserve
-    compare_new_drop(
-        cl.Development(drop_above=1.01, drop_below=0.95).fit(clrd), clrd
-    )
+    compare_new_drop(cl.Development(drop_above=1.01, drop_below=0.95).fit(clrd), clrd)
 
 
 def test_new_drop_7(clrd):