From b54fb9823abeece117cc5bd6021f78e5487adb7f Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Fri, 9 Dec 2022 13:13:22 +0000 Subject: [PATCH 1/9] Added conditional property to pipeline to expose time scale prediction models --- sksurv/__init__.py | 7 ++++ sksurv/util.py | 89 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) diff --git a/sksurv/__init__.py b/sksurv/__init__.py index 76671fd6..8410a142 100644 --- a/sksurv/__init__.py +++ b/sksurv/__init__.py @@ -5,6 +5,7 @@ from pkg_resources import DistributionNotFound, get_distribution from sklearn.pipeline import Pipeline, _final_estimator_has from sklearn.utils.metaestimators import available_if +from .util import conditionalAvailableProperty def _get_version(name): @@ -128,9 +129,15 @@ def predict_survival_function(self, X, **kwargs): return self.steps[-1][-1].predict_survival_function(Xt, **kwargs) +@conditionalAvailableProperty(_final_estimator_has('_predict_risk_score')) +def _predict_risk_score(self): + return self.steps[-1][-1]._predict_risk_score + + def patch_pipeline(): Pipeline.predict_survival_function = predict_survival_function Pipeline.predict_cumulative_hazard_function = predict_cumulative_hazard_function + Pipeline._predict_risk_score = _predict_risk_score try: diff --git a/sksurv/util.py b/sksurv/util.py index bb33eda8..3e5fab5a 100644 --- a/sksurv/util.py +++ b/sksurv/util.py @@ -260,3 +260,92 @@ def safe_concat(objs, *args, **kwargs): concatenated[name] = pd.Categorical(concatenated[name], **params) return concatenated + + +class _ConditionalAvailableProperty: + """Implements a conditional property using the descriptor protocol based on the property decorator. + + The corresponding class in scikit-learn (_AvailableIfDescriptor) only supports callables. This + class adopts the property decorator as presented by the descriptor guide in the offical documentation. + + The check is defined on the getter function. Setter, Deleter also need to be defined on the exposing class + if they are supposed to be available. + + See Also + -------- + sklearn.utils.available_if._AvailableIfDescriptor: + The original class in scikit-learn. + """ + + def __init__(self, check, fget=None, fset=None, fdel=None, doc=None): + self._check = check + self.fget = fget + self.fset = fset + self.fdel = fdel + if doc is None and fget is not None: + doc = fget.__doc__ + self.__doc__ = doc + self._name = '' + + def _run_check(self, obj): + attr_err = AttributeError( + f"This {repr(obj)} has no attribute {repr(self._name)}" + ) + if not self.check(obj): + raise attr_err + + def __set_name__(self, owner, name): + self._name = name + + def __get__(self, obj, objtype=None): + if obj is None: + return self + self._run_check(obj) + if self.fget is None: + raise AttributeError(f"property '{self._name}' has no getter") + return self.fget(obj) + + def __set__(self, obj, value): + self._run_check(obj) + if self.fset is None: + raise AttributeError(f"property '{self._name}' has no setter") + self.fset(obj, value) + + def __delete__(self, obj): + self._run_check(obj) + if self.fdel is None: + raise AttributeError(f"property '{self._name}' has no deleter") + self.fdel(obj) + + @property + def check(self): + return self._check + + @check.setter + def check(self, check): + if self._check is not None: + raise AttributeError(f"property '{self._name}' already had a check set") + if check is None: + return self._check + else: + return check + + def getter(self, fget): + prop = type(self)(self.check, fget, self.fset, self.fdel, self.__doc__) + prop._name = self._name + return prop + + def setter(self, fset): + prop = type(self)(self.check, self.fget, fset, self.fdel, self.__doc__) + prop._name = self._name + return prop + + def deleter(self, fdel): + prop = type(self)(self.check, self.fget, self.fset, fdel, self.__doc__) + prop._name = self._name + return prop + + +def conditionalAvailableProperty(check): + prop = _ConditionalAvailableProperty(check=check) + return prop.getter From 7aa910c6fcae495083c97304d52da2f439f838b8 Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Fri, 9 Dec 2022 13:42:00 +0000 Subject: [PATCH 2/9] Removed unnecessary else --- sksurv/util.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sksurv/util.py b/sksurv/util.py index 3e5fab5a..c16e82d3 100644 --- a/sksurv/util.py +++ b/sksurv/util.py @@ -327,8 +327,7 @@ def check(self, check): raise AttributeError(f"property '{self._name}' already had a check set") if check is None: return self._check - else: - return check + return check def getter(self, fget): prop = type(self)(self.check, fget, self.fset, self.fdel, self.__doc__) From 3ec982d5573f982f78e62a82678d8275eab24a72 Mon Sep 17 00:00:00 2001 From: Lukas Jansen Date: Fri, 3 Mar 2023 13:06:30 +0100 Subject: [PATCH 3/9] Sorted imports --- sksurv/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sksurv/__init__.py b/sksurv/__init__.py index 8410a142..935afc70 100644 --- a/sksurv/__init__.py +++ b/sksurv/__init__.py @@ -5,6 +5,7 @@ from pkg_resources import DistributionNotFound, get_distribution from sklearn.pipeline import Pipeline, _final_estimator_has from sklearn.utils.metaestimators import available_if + from .util import conditionalAvailableProperty From ccd891d3abe8e670efbdee81ff0c48e56a60cd65 Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Tue, 11 Apr 2023 08:32:52 +0000 Subject: [PATCH 4/9] Added tests --- tests/test_util.py | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/test_util.py b/tests/test_util.py index cb66b198..9a3b75b0 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -8,7 +8,7 @@ import pytest from sksurv.testing import FixtureParameterFactory -from sksurv.util import Surv, safe_concat +from sksurv.util import Surv, safe_concat, conditionalAvailableProperty class ConcatCasesFactory(FixtureParameterFactory): @@ -378,3 +378,44 @@ def test_from_dataframe(args, expected, expected_error): if expected is not None: assert_array_equal(y, expected) + +def test_cond_avail_property(): + class WithCondProp: + def __init__(self, val): + self.avail = False + self._prop = val + + @conditionalAvailableProperty(lambda self: self.avail) + def prop(self): + return self._prop + + @prop.setter + def prop(self, new): + self._prop = new + + @prop.deleter + def prop(self): + self.avail = False + + testval = 43 + msg = "has no attribute 'prop'" + + test_obj = WithCondProp(testval) + with pytest.raises(AttributeError, match=msg): + test_obj.prop + with pytest.raises(AttributeError, match=msg): + test_obj.prop = testval-1 + with pytest.raises(AttributeError, match=msg): + del test_obj.prop + test_obj.avail = True + assert test_obj.prop == testval + test_obj.prop = testval-2 + assert test_obj.prop == testval-2 + del test_obj.prop + assert test_obj.avail == False + with pytest.raises(AttributeError, match=msg): + test_obj.prop + with pytest.raises(AttributeError, match=msg): + test_obj.prop = testval-3 + with pytest.raises(AttributeError, match=msg): + del test_obj.prop From 62700390aac6bdb320dc03425381353a0f55810e Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Tue, 11 Apr 2023 08:41:38 +0000 Subject: [PATCH 5/9] Tried to fix Codacy issues --- tests/test_util.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_util.py b/tests/test_util.py index 9a3b75b0..ed03fcc3 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -399,10 +399,10 @@ def prop(self): testval = 43 msg = "has no attribute 'prop'" - + test_obj = WithCondProp(testval) with pytest.raises(AttributeError, match=msg): - test_obj.prop + _ = test_obj.prop with pytest.raises(AttributeError, match=msg): test_obj.prop = testval-1 with pytest.raises(AttributeError, match=msg): @@ -412,9 +412,9 @@ def prop(self): test_obj.prop = testval-2 assert test_obj.prop == testval-2 del test_obj.prop - assert test_obj.avail == False + assert test_obj.avail is False with pytest.raises(AttributeError, match=msg): - test_obj.prop + _ = test_obj.prop with pytest.raises(AttributeError, match=msg): test_obj.prop = testval-3 with pytest.raises(AttributeError, match=msg): From 1e127582f80a4b5a31b2592689d23afe51dbc91e Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Fri, 14 Apr 2023 09:45:47 +0000 Subject: [PATCH 6/9] Improved test ccv --- sksurv/util.py | 4 +--- tests/test_aft.py | 18 ++++++++++++++++++ tests/test_util.py | 19 ++++++++++++++++++- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/sksurv/util.py b/sksurv/util.py index c16e82d3..674889b3 100644 --- a/sksurv/util.py +++ b/sksurv/util.py @@ -325,9 +325,7 @@ def check(self): def check(self, check): if self._check is not None: raise AttributeError(f"property '{self._name}' already had a check set") - if check is None: - return self._check - return check + self._check = check def getter(self, fget): prop = type(self)(self.check, fget, self.fset, self.fdel, self.__doc__) diff --git a/tests/test_aft.py b/tests/test_aft.py index 93884e5b..51117f66 100644 --- a/tests/test_aft.py +++ b/tests/test_aft.py @@ -1,6 +1,8 @@ import numpy as np from numpy.testing import assert_array_almost_equal +from sklearn.pipeline import make_pipeline +from sksurv.base import SurvivalAnalysisMixin from sksurv.linear_model import IPCRidge from sksurv.testing import assert_cindex_almost_equal @@ -36,3 +38,19 @@ def test_predict(make_whas500): ) assert model.score(x_test, y_test) == 0.66925817946226107 + + @staticmethod + def test_pipeline_score(make_whas500): + whas500 = make_whas500() + pipe = make_pipeline(IPCRidge()) + pipe.fit(whas500.x[:400], whas500.y[:400]) + + x_test = whas500.x[400:] + y_test = whas500.y[400:] + p = pipe.predict(x_test) + assert_cindex_almost_equal( + y_test['fstat'], y_test['lenfol'], -p, + (0.66925817946226107, 2066, 1021, 0, 1), + ) + + assert SurvivalAnalysisMixin.score(pipe, x_test, y_test) == 0.66925817946226107 diff --git a/tests/test_util.py b/tests/test_util.py index ed03fcc3..334b650f 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -8,7 +8,7 @@ import pytest from sksurv.testing import FixtureParameterFactory -from sksurv.util import Surv, safe_concat, conditionalAvailableProperty +from sksurv.util import Surv, conditionalAvailableProperty, safe_concat class ConcatCasesFactory(FixtureParameterFactory): @@ -400,6 +400,8 @@ def prop(self): testval = 43 msg = "has no attribute 'prop'" + assert WithCondProp.prop is not None + test_obj = WithCondProp(testval) with pytest.raises(AttributeError, match=msg): _ = test_obj.prop @@ -419,3 +421,18 @@ def prop(self): test_obj.prop = testval-3 with pytest.raises(AttributeError, match=msg): del test_obj.prop + + test_obj.avail = True + WithCondProp.prop.fget = None + with pytest.raises(AttributeError, match='has no getter'): + _ = test_obj.prop + WithCondProp.prop.fset = None + with pytest.raises(AttributeError, match='has no setter'): + test_obj.prop = testval-4 + WithCondProp.prop.fdel = None + with pytest.raises(AttributeError, match='has no deleter'): + del test_obj.prop + + with pytest.raises(AttributeError, match='already had a check set'): + oldcheck = test_obj.check + test_obj.prop.check = oldcheck From c15832ed7fa651ef25ffc44958f55444cb786975 Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Fri, 14 Apr 2023 09:53:59 +0000 Subject: [PATCH 7/9] Fixed test --- tests/test_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_util.py b/tests/test_util.py index 334b650f..55fe1e19 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -434,5 +434,5 @@ def prop(self): del test_obj.prop with pytest.raises(AttributeError, match='already had a check set'): - oldcheck = test_obj.check + oldcheck = test_obj.prop.check test_obj.prop.check = oldcheck From dac2c18fba5611c6475f1ebc466a63b217226353 Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Fri, 14 Apr 2023 09:57:36 +0000 Subject: [PATCH 8/9] Removed setter for check --- sksurv/util.py | 6 ------ tests/test_util.py | 6 ++---- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/sksurv/util.py b/sksurv/util.py index 674889b3..a327d7f0 100644 --- a/sksurv/util.py +++ b/sksurv/util.py @@ -321,12 +321,6 @@ def __delete__(self, obj): def check(self): return self._check - @check.setter - def check(self, check): - if self._check is not None: - raise AttributeError(f"property '{self._name}' already had a check set") - self._check = check - def getter(self, fget): prop = type(self)(self.check, fget, self.fset, self.fdel, self.__doc__) prop._name = self._name diff --git a/tests/test_util.py b/tests/test_util.py index 55fe1e19..18dd71e6 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -421,7 +421,7 @@ def prop(self): test_obj.prop = testval-3 with pytest.raises(AttributeError, match=msg): del test_obj.prop - + test_obj.avail = True WithCondProp.prop.fget = None with pytest.raises(AttributeError, match='has no getter'): @@ -433,6 +433,4 @@ def prop(self): with pytest.raises(AttributeError, match='has no deleter'): del test_obj.prop - with pytest.raises(AttributeError, match='already had a check set'): - oldcheck = test_obj.prop.check - test_obj.prop.check = oldcheck + From 7a8e2f4f317e8488916defe5b0336997f9db6356 Mon Sep 17 00:00:00 2001 From: Finesim97 Date: Mon, 15 May 2023 10:57:02 +0000 Subject: [PATCH 9/9] WS not noticed by tox locally removed --- tests/test_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_util.py b/tests/test_util.py index 18dd71e6..3b8a0493 100644 --- a/tests/test_util.py +++ b/tests/test_util.py @@ -421,7 +421,7 @@ def prop(self): test_obj.prop = testval-3 with pytest.raises(AttributeError, match=msg): del test_obj.prop - + test_obj.avail = True WithCondProp.prop.fget = None with pytest.raises(AttributeError, match='has no getter'):