10000 Merge pull request #541 from ogrisel/immutable-readonly-coef · scikit-learn/scikit-learn@26632b6 · GitHub
[go: up one dir, main page]

Skip to content

Commit 26632b6

Browse files
committed
Merge pull request #541 from ogrisel/immutable-readonly-coef
MRG: Explicitly mark the array returned by the linear svm readonly property coef_ as immutable
2 parents d869bf3 + da5d1d8 commit 26632b6

File tree

4 files changed

+65
-5
lines changed

4 files changed

+65
-5
lines changed

sklearn/linear_model/logistic.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ class LogisticRegression(BaseLibLinear, ClassifierMixin,
5858
`coef_` : array, shape = [n_classes-1, n_features]
5959
Coefficient of the features in the decision function.
6060
61+
`coef_` is readonly property derived from `raw_coef_` that
62+
follows the internal memory layout of liblinear.
63+
6164
`intercept_` : array, shape = [n_classes-1]
6265
intercept (a.k.a. bias) added to the decision function.
6366
It is available only when parameter intercept is set to True

sklearn/svm/base.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
from abc import ABCMeta, abstractmethod
22

33
import numpy as np
4+
import scipy.sparse as sp
45

56
from . import libsvm, liblinear
67
from ..base import BaseEstimator
7-
from ..utils import array2d, safe_asarray
8+
from ..utils import array2d
9+
from ..utils import safe_asarray
10+
from ..utils.extmath import safe_sparse_dot
811
import warnings
912

1013
dot = np.dot
@@ -118,7 +121,16 @@ def coef_(self):
118121
if self.kernel != 'linear':
119122
raise NotImplementedError('coef_ is only available when using a '
120123
'linear kernel')
121-
return dot(self.dual_coef_, self.support_vectors_)
124+
coef = safe_sparse_dot(self.dual_coef_, self.support_vectors_)
125+
# coef_ being a read-only property it's better to mark the value as
126+
# immutable to avoid hiding potential bugs for the unsuspecting user
127+
if sp.issparse(coef):
128+
# sparse matrix do not have global flags
129+
coef.data.flags.writeable = False
130+
else:
131+
# regular dense array
132+
coef.flags.writeable = False
133+
return coef
122134

123135

124136
class DenseBaseLibSVM(BaseLibSVM):
@@ -525,12 +537,18 @@ def intercept_(self):
525537
@property
526538
def coef_(self):
527539
if self.fit_intercept:
528-
ret = self.raw_coef_[:, : -1]
540+
ret = self.raw_coef_[:, : -1].copy()
529541
else:
530-
ret = self.raw_coef_
542+
ret = self.raw_coef_.copy()
543+
544+
# as coef_ is readonly property, mark the returned value as immutable
545+
# to avoid silencing potential bugs
531546
if len(self.label_) <= 2:
532-
return -ret
547+
ret *= -1
548+
ret.flags.writeable = False
549+
return ret
533550
else:
551+
ret.flags.writeable = False
534552
return ret
535553

536554
def _get_bias(self):

sklearn/svm/classes.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ class LinearSVC(BaseLibLinear, ClassifierMixin, CoefSelectTransformerMixin):
6363
Weights asigned to the features (coefficients in the primal
6464
problem). This is only available in the case of linear kernel.
6565
66+
`coef_` is readonly property derived from `raw_coef_` that
67+
follows the internal memory layout of liblinear.
68+
6669
`intercept_` : array, shape = [1] if n_classes == 2 else [n_classes]
6770
Constants in decision function.
6871
@@ -146,6 +149,9 @@ class SVC(DenseBaseLibSVM, ClassifierMixin):
146149
Weights asigned to the features (coefficients in the primal
147150
problem). This is only available in the case of linear kernel.
148151
152+
`coef_` is readonly property derived from `dual_coef_` and
153+
`support_vectors_`
154+
149155
`intercept_` : array, shape = [n_class * (n_class-1) / 2]
150156
Constants in decision function.
151157
@@ -234,6 +240,9 @@ class NuSVC(DenseBaseLibSVM, ClassifierMixin):
234240
Weights asigned to the features (coefficients in the primal
235241
problem). This is only available in the case of linear kernel.
236242
243+
`coef_` is readonly property derived from `dual_coef_` and
244+
`support_vectors_`
245+
237246
`intercept_` : array, shape = [n_class * (n_class-1) / 2]
238247
Constants in decision function.
239248
@@ -329,6 +338,9 @@ class SVR(DenseBaseLibSVM, RegressorMixin):
329338
Weights asigned to the features (coefficients in the primal
330339
problem). This is only available in the case of linear kernel.
331340
341+
`coef_` is readonly property derived from `dual_coef_` and
342+
`support_vectors_`
343+
332344
`intercept_` : array, shape = [n_class * (n_class-1) / 2]
333345
Constants in decision function.
334346
@@ -449,6 +461,9 @@ class NuSVR(DenseBaseLibSVM, RegressorMixin):
449461
Weights asigned to the features (coefficients in the primal
450462
problem). This is only available in the case of linear kernel.
451463
464+
`coef_` is readonly property derived from `dual_coef_` and
465+
`support_vectors_`
466+
452467
`intercept_` : array, shape = [n_class * (n_class-1) / 2]
453468
Constants in decision function.
454469
@@ -558,6 +573,9 @@ class OneClassSVM(DenseBaseLibSVM):
558573
Weights asigned to the features (coefficients in the primal
559574
problem). This is only available in the case of linear kernel.
560575
576+
`coef_` is readonly property derived from `dual_coef_` and
577+
`support_vectors_`
578+
561579
`intercept_` : array, shape = [n_classes-1]
562580
Constants in decision function.
563581
@@ -592,3 +610,4 @@ def fit(self, X, class_weight={}, sample_weight=None, **params):
592610
super(OneClassSVM, self).fit(
593611
X, [], class_weight=class_weight, sample_weight=sample_weight,
594612
**params)
613+
return self

sklearn/svm/tests/test_svm.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,26 @@ def test_nu_svc_samples_scaling():
531531
assert_true(error_with_scale < 1e-5)
532532

533533

534+
def test_immutable_coef_property():
535+
"""Check that primal coef modification are not silently ignored"""
536+
svms = [
537+
svm.SVC(kernel='linear').fit(iris.data, iris.target),
538+
svm.NuSVC(kernel='linear').fit(iris.data, iris.target),
539+
svm.SVR(kernel='linear').fit(iris.data, iris.target),
540+
svm.NuSVR(kernel='linear').fit(iris.data, iris.target),
541+
svm.OneClassSVM(kernel='linear').fit(iris.data),
542+
svm.sparse.SVC(kernel='linear').fit(iris.data, iris.target),
543+
svm.sparse.NuSVC(kernel='linear').fit(iris.data, iris.target),
544+
svm.sparse.SVR(kernel='linear').fit(iris.data, iris.target),
545+
svm.sparse.NuSVR(kernel='linear').fit(iris.data, iris.target),
546+
svm.LinearSVC().fit(iris.data, iris.target),
547+
linear_model.LogisticRegression().fit(iris.data, iris.target),
548+
]
549+
for clf in svms:
550+
assert_raises(AttributeError, clf.__setattr__, 'coef_', np.arange(3))
551+
assert_raises(RuntimeError, clf.coef_.__setitem__, (0, 0), 0)
552+
553+
534554
if __name__ == '__main__':
535555
import nose
536556
nose.runmodule()

0 commit comments

Comments
 (0)
0