[enhancement] ban runtime use of Python's property in sklearnex (…

…uxlfoundation#2102)

onedal/cluster/kmeans.py

@@ -349,7 +349,8 @@ def is_better_iteration(inertia, labels):
 
         return self
 
-    def _get_cluster_centers(self):
+    @property
+    def cluster_centers_(self):
         if not hasattr(self, "_cluster_centers_"):
             if hasattr(self, "model_"):
                 centroids = self.model_.centroids
@@ -358,7 +359,8 @@ def _get_cluster_centers(self):
                 raise NameError("This model have not been trained")
         return self._cluster_centers_
 
-    def _set_cluster_centers(self, cluster_centers):
+    @cluster_centers_.setter
+    def cluster_centers_(self, cluster_centers):
         self._cluster_centers_ = np.asarray(cluster_centers)
 
         self.n_iter_ = 0
@@ -371,7 +373,9 @@ def _set_cluster_centers(self, cluster_centers):
 
         return self
 
-    cluster_centers_ = property(_get_cluster_centers, _set_cluster_centers)
+    @cluster_centers_.deleter
+    def cluster_centers_(self):
+        del self._cluster_centers_
 
     def _predict(self, X, module, queue=None, result_options=None):
         is_csr = _is_csr(X)

sklearnex/linear_model/linear.py

@@ -278,31 +278,41 @@ def _onedal_score(self, X, y, sample_weight=None, queue=None):
             y, self._onedal_predict(X, queue=queue), sample_weight=sample_weight
         )
 
-    def get_coef_(self):
-        return self.coef_
+    @property
+    def coef_(self):
+        return self._coef_
 
-    def set_coef_(self, value):
-        self.__dict__["coef_"] = value
+    @coef_.setter
+    def coef_(self, value):
+        self._coef_ = value
         if hasattr(self, "_onedal_estimator"):
             self._onedal_estimator.coef_ = value
             del self._onedal_estimator._onedal_model
 
-    def get_intercept_(self):
-        return self.intercept_
+    @coef_.deleter
+    def coef_(self):
+        del self._coef_
 
-    def set_intercept_(self, value):
-        self.__dict__["intercept_"] = value
+    @property
+    def intercept_(self):
+        return self._intercept_
+
+    @intercept_.setter
+    def intercept_(self, value):
+        self._intercept_ = value
         if hasattr(self, "_onedal_estimator"):
             self._onedal_estimator.intercept_ = value
             del self._onedal_estimator._onedal_model
 
+    @intercept_.deleter
+    def intercept_(self):
+        del self._intercept_
+
     def _save_attributes(self):
-        self.coef_ = property(self.get_coef_, self.set_coef_)
-        self.intercept_ = property(self.get_intercept_, self.set_intercept_)
         self.n_features_in_ = self._onedal_estimator.n_features_in_
         self._sparse = False
-        self.__dict__["coef_"] = self._onedal_estimator.coef_
-        self.__dict__["intercept_"] = self._onedal_estimator.intercept_
+        self._coef_ = self._onedal_estimator.coef_
+        self._intercept_ = self._onedal_estimator.intercept_
 
     fit.__doc__ = _sklearn_LinearRegression.fit.__doc__
     predict.__doc__ = _sklearn_LinearRegression.predict.__doc__

sklearnex/svm/_common.py

@@ -37,32 +37,40 @@
     validate_data = BaseEstimator._validate_data
 
 
-def get_dual_coef(self):
-    return self.dual_coef_
-
-
-def set_dual_coef(self, value):
-    self.dual_coef_ = value
-    if hasattr(self, "_onedal_estimator"):
-        self._onedal_estimator.dual_coef_ = value
-        if not self._is_in_fit:
-            del self._onedal_estimator._onedal_model
-
-
-def get_intercept(self):
-    return self._intercept_
-
-
-def set_intercept(self, value):
-    self._intercept_ = value
-    if hasattr(self, "_onedal_estimator"):
-        self._onedal_estimator.intercept_ = value
-        if not self._is_in_fit:
-            del self._onedal_estimator._onedal_model
-
-
 class BaseSVM(BaseEstimator, ABC):
 
+    @property
+    def _dual_coef_(self):
+        return self._dualcoef_
+
+    @_dual_coef_.setter
+    def _dual_coef_(self, value):
+        self._dualcoef_ = value
+        if hasattr(self, "_onedal_estimator"):
+            self._onedal_estimator.dual_coef_ = value
+            if hasattr(self._onedal_estimator, "_onedal_model"):
+                del self._onedal_estimator._onedal_model
+
+    @_dual_coef_.deleter
+    def _dual_coef_(self):
+        del self._dualcoef_
+
+    @property
+    def intercept_(self):
+        return self._icept_
+
+    @intercept_.setter
+    def intercept_(self, value):
+        self._icept_ = value
+        if hasattr(self, "_onedal_estimator"):
+            self._onedal_estimator.intercept_ = value
+            if hasattr(self._onedal_estimator, "_onedal_model"):
+                del self._onedal_estimator._onedal_model
+
+    @intercept_.deleter
+    def intercept_(self):
+        del self._icept_
+
     def _onedal_gpu_supported(self, method_name, *data):
         patching_status = PatchingConditionsChain(f"sklearn.{method_name}")
         patching_status.and_conditions([(False, "GPU offloading is not supported.")])
@@ -285,7 +293,7 @@ def _save_attributes(self):
             self.class_weight_ = self._onedal_estimator.class_weight_
         self.support_ = self._onedal_estimator.support_
 
-        self._intercept_ = self._onedal_estimator.intercept_
+        self._icept_ = self._onedal_estimator.intercept_
         self._n_support = self._onedal_estimator._n_support
         self._sparse = False
         self._gamma = self._onedal_estimator._gamma
@@ -297,13 +305,7 @@ def _save_attributes(self):
             self._probA = np.empty(0)
             self._probB = np.empty(0)
 
-        self._dual_coef_ = property(get_dual_coef, set_dual_coef)
-        self.intercept_ = property(get_intercept, set_intercept)
-
-        self._is_in_fit = True
-        self._dual_coef_ = self.dual_coef_
-        self.intercept_ = self._intercept_
-        self._is_in_fit = False
+        self._dualcoef_ = self.dual_coef_
 
         if sklearn_check_version("1.1"):
             length = int(len(self.classes_) * (len(self.classes_) - 1) / 2)
@@ -319,24 +321,18 @@ def _save_attributes(self):
         self.shape_fit_ = self._onedal_estimator.shape_fit_
         self.support_ = self._onedal_estimator.support_
 
-        self._intercept_ = self._onedal_estimator.intercept_
+        self._icept_ = self._onedal_estimator.intercept_
         self._n_support = [self.support_vectors_.shape[0]]
         self._sparse = False
         self._gamma = self._onedal_estimator._gamma
         self._probA = None
         self._probB = None
 
-        self._dual_coef_ = property(get_dual_coef, set_dual_coef)
-        self.intercept_ = property(get_intercept, set_intercept)
-
-        self._is_in_fit = True
-        self._dual_coef_ = self.dual_coef_
-        self.intercept_ = self._intercept_
-        self._is_in_fit = False
-
         if sklearn_check_version("1.1"):
             self.n_iter_ = self._onedal_estimator.n_iter_
 
+        self._dualcoef_ = self.dual_coef_
+
     def _onedal_score(self, X, y, sample_weight=None, queue=None):
         return r2_score(
             y, self._onedal_predict(X, queue=queue), sample_weight=sample_weight

sklearnex/tests/test_common.py

@@ -339,7 +339,14 @@ def n_jobs_check(text, estimator, method):
     ), f"verify if {method} should be in control_n_jobs' decorated_methods for {estimator}"
 
 
-DESIGN_RULES = [n_jobs_check]
+def runtime_property_check(text, estimator, method):
+    """use of Python's 'property' should not be used at runtime, only at class instantiation"""
+    assert (
+        len(re.findall(r"property\(", text[1])) == 0
+    ), f"{estimator}.{method} should only use 'property' at instantiation"
+
+
+DESIGN_RULES = [n_jobs_check, runtime_property_check]
 
 
 if sklearn_check_version("1.0"):