From bf1ccbe8f34801d3e1b615b400b010eaf0b5e9c9 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 6 May 2023 20:17:43 +0200 Subject: [PATCH 1/2] feat: mark optional hyperparameters as keyword-only --- src/safeds/ml/classical/classification/_ada_boost.py | 1 + src/safeds/ml/classical/classification/_gradient_boosting.py | 2 +- src/safeds/ml/classical/classification/_random_forest.py | 4 ++-- .../ml/classical/classification/_support_vector_machine.py | 4 ++-- src/safeds/ml/classical/regression/_ada_boost.py | 1 + src/safeds/ml/classical/regression/_elastic_net_regression.py | 2 +- src/safeds/ml/classical/regression/_gradient_boosting.py | 2 +- src/safeds/ml/classical/regression/_lasso_regression.py | 2 +- src/safeds/ml/classical/regression/_random_forest.py | 4 ++-- src/safeds/ml/classical/regression/_ridge_regression.py | 2 +- src/safeds/ml/classical/regression/_support_vector_machine.py | 4 ++-- 11 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/safeds/ml/classical/classification/_ada_boost.py b/src/safeds/ml/classical/classification/_ada_boost.py index 1b08b1b1b..cf2d4883b 100644 --- a/src/safeds/ml/classical/classification/_ada_boost.py +++ b/src/safeds/ml/classical/classification/_ada_boost.py @@ -37,6 +37,7 @@ class AdaBoost(Classifier): def __init__( self, + *, learner: Classifier | None = None, maximum_number_of_learners: int = 50, learning_rate: float = 1.0, diff --git a/src/safeds/ml/classical/classification/_gradient_boosting.py b/src/safeds/ml/classical/classification/_gradient_boosting.py index 4db4b2c19..4b7cb9cd3 100644 --- a/src/safeds/ml/classical/classification/_gradient_boosting.py +++ b/src/safeds/ml/classical/classification/_gradient_boosting.py @@ -33,7 +33,7 @@ class GradientBoosting(Classifier): If `number_of_trees` is less than or equal to 0 or `learning_rate` is non-positive. """ - def __init__(self, number_of_trees: int = 100, learning_rate: float = 0.1) -> None: + def __init__(self, *, number_of_trees: int = 100, learning_rate: float = 0.1) -> None: # Validation if number_of_trees <= 0: raise ValueError("The parameter 'number_of_trees' has to be greater than 0.") diff --git a/src/safeds/ml/classical/classification/_random_forest.py b/src/safeds/ml/classical/classification/_random_forest.py index eb4ccc6f8..1bc77606d 100644 --- a/src/safeds/ml/classical/classification/_random_forest.py +++ b/src/safeds/ml/classical/classification/_random_forest.py @@ -28,7 +28,7 @@ class RandomForest(Classifier): If `number_of_trees` is less than or equal to 0. """ - def __init__(self, number_of_trees: int = 100) -> None: + def __init__(self, *, number_of_trees: int = 100) -> None: # Validation if number_of_trees < 1: raise ValueError("The parameter 'number_of_trees' has to be greater than 0.") @@ -65,7 +65,7 @@ def fit(self, training_set: TaggedTable) -> RandomForest: wrapped_classifier = self._get_sklearn_classifier() fit(wrapped_classifier, training_set) - result = RandomForest(self._number_of_trees) + result = RandomForest(number_of_trees=self._number_of_trees) result._wrapped_classifier = wrapped_classifier result._feature_names = training_set.features.column_names result._target_name = training_set.target.name diff --git a/src/safeds/ml/classical/classification/_support_vector_machine.py b/src/safeds/ml/classical/classification/_support_vector_machine.py index b6d7f304a..c9e2e958e 100644 --- a/src/safeds/ml/classical/classification/_support_vector_machine.py +++ b/src/safeds/ml/classical/classification/_support_vector_machine.py @@ -29,7 +29,7 @@ class SupportVectorMachine(Classifier): If `c` is less than or equal to 0. """ - def __init__(self, c: float = 1.0) -> None: + def __init__(self, *, c: float = 1.0) -> None: # Internal state self._wrapped_classifier: sk_SVC | None = None self._feature_names: list[str] | None = None @@ -63,7 +63,7 @@ def fit(self, training_set: TaggedTable) -> SupportVectorMachine: wrapped_classifier = self._get_sklearn_classifier() fit(wrapped_classifier, training_set) - result = SupportVectorMachine(self._c) + result = SupportVectorMachine(c=self._c) result._wrapped_classifier = wrapped_classifier result._feature_names = training_set.features.column_names result._target_name = training_set.target.name diff --git a/src/safeds/ml/classical/regression/_ada_boost.py b/src/safeds/ml/classical/regression/_ada_boost.py index 94a88142b..fcf5e553b 100644 --- a/src/safeds/ml/classical/regression/_ada_boost.py +++ b/src/safeds/ml/classical/regression/_ada_boost.py @@ -37,6 +37,7 @@ class AdaBoost(Regressor): def __init__( self, + *, learner: Regressor | None = None, maximum_number_of_learners: int = 50, learning_rate: float = 1.0, diff --git a/src/safeds/ml/classical/regression/_elastic_net_regression.py b/src/safeds/ml/classical/regression/_elastic_net_regression.py index 14a549e53..67308c51a 100644 --- a/src/safeds/ml/classical/regression/_elastic_net_regression.py +++ b/src/safeds/ml/classical/regression/_elastic_net_regression.py @@ -33,7 +33,7 @@ class ElasticNetRegression(Regressor): If `alpha` is negative or `lasso_ratio` is not between 0 and 1. """ - def __init__(self, alpha: float = 1.0, lasso_ratio: float = 0.5) -> None: + def __init__(self, *, alpha: float = 1.0, lasso_ratio: float = 0.5) -> None: # Validation if alpha < 0: raise ValueError("The parameter 'alpha' must be non-negative") diff --git a/src/safeds/ml/classical/regression/_gradient_boosting.py b/src/safeds/ml/classical/regression/_gradient_boosting.py index 4889f72cf..749a78836 100644 --- a/src/safeds/ml/classical/regression/_gradient_boosting.py +++ b/src/safeds/ml/classical/regression/_gradient_boosting.py @@ -33,7 +33,7 @@ class GradientBoosting(Regressor): If `number_of_trees` is less than or equal to 0 or `learning_rate` is non-positive. """ - def __init__(self, number_of_trees: int = 100, learning_rate: float = 0.1) -> None: + def __init__(self, *, number_of_trees: int = 100, learning_rate: float = 0.1) -> None: # Validation if number_of_trees <= 0: raise ValueError("The parameter 'number_of_trees' has to be greater than 0.") diff --git a/src/safeds/ml/classical/regression/_lasso_regression.py b/src/safeds/ml/classical/regression/_lasso_regression.py index 89aeeae22..0269908b3 100644 --- a/src/safeds/ml/classical/regression/_lasso_regression.py +++ b/src/safeds/ml/classical/regression/_lasso_regression.py @@ -29,7 +29,7 @@ class LassoRegression(Regressor): If `alpha` is negative. """ - def __init__(self, alpha: float = 1.0) -> None: + def __init__(self, *, alpha: float = 1.0) -> None: # Validation if alpha < 0: raise ValueError("The parameter 'alpha' must be non-negative") diff --git a/src/safeds/ml/classical/regression/_random_forest.py b/src/safeds/ml/classical/regression/_random_forest.py index 85fbef720..3416a1029 100644 --- a/src/safeds/ml/classical/regression/_random_forest.py +++ b/src/safeds/ml/classical/regression/_random_forest.py @@ -28,7 +28,7 @@ class RandomForest(Regressor): If `number_of_trees` is less than or equal to 0. """ - def __init__(self, number_of_trees: int = 100) -> None: + def __init__(self, *, number_of_trees: int = 100) -> None: # Validation if number_of_trees < 1: raise ValueError("The parameter 'number_of_trees' has to be greater than 0.") @@ -65,7 +65,7 @@ def fit(self, training_set: TaggedTable) -> RandomForest: wrapped_regressor = self._get_sklearn_regressor() fit(wrapped_regressor, training_set) - result = RandomForest(self._number_of_trees) + result = RandomForest(number_of_trees=self._number_of_trees) result._wrapped_regressor = wrapped_regressor result._feature_names = training_set.features.column_names result._target_name = training_set.target.name diff --git a/src/safeds/ml/classical/regression/_ridge_regression.py b/src/safeds/ml/classical/regression/_ridge_regression.py index 489e13fef..eda5ff0ba 100644 --- a/src/safeds/ml/classical/regression/_ridge_regression.py +++ b/src/safeds/ml/classical/regression/_ridge_regression.py @@ -30,7 +30,7 @@ class RidgeRegression(Regressor): If `alpha` is negative. """ - def __init__(self, alpha: float = 1.0) -> None: + def __init__(self, *, alpha: float = 1.0) -> None: # Validation if alpha < 0: raise ValueError("The parameter 'alpha' must be non-negative") diff --git a/src/safeds/ml/classical/regression/_support_vector_machine.py b/src/safeds/ml/classical/regression/_support_vector_machine.py index 1f044212c..41a194d96 100644 --- a/src/safeds/ml/classical/regression/_support_vector_machine.py +++ b/src/safeds/ml/classical/regression/_support_vector_machine.py @@ -29,7 +29,7 @@ class SupportVectorMachine(Regressor): If `c` is less than or equal to 0. """ - def __init__(self, c: float = 1.0) -> None: + def __init__(self, *, c: float = 1.0) -> None: # Internal state self._wrapped_regressor: sk_SVR | None = None self._feature_names: list[str] | None = None @@ -63,7 +63,7 @@ def fit(self, training_set: TaggedTable) -> SupportVectorMachine: wrapped_regressor = self._get_sklearn_regressor() fit(wrapped_regressor, training_set) - result = SupportVectorMachine(self._c) + result = SupportVectorMachine(c=self._c) result._wrapped_regressor = wrapped_regressor result._feature_names = training_set.features.column_names result._target_name = training_set.target.name From 5582d241063dddc021e5f2869596f76392afc882 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sat, 6 May 2023 20:27:59 +0200 Subject: [PATCH 2/2] docs: add guideline --- docs/development/guidelines.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/development/guidelines.md b/docs/development/guidelines.md index 2a85ecea9..707110c8e 100644 --- a/docs/development/guidelines.md +++ b/docs/development/guidelines.md @@ -72,6 +72,26 @@ Write full words rather than abbreviations. The increased verbosity is offset by figure.scs(CS.AUT) ``` +### Consider marking optional parameters as keyword-only + +_Keyword-only parameters_ are parameters that can only be passed by name. It prevents users from accidentally passing a value to the wrong parameter. This can happen easily if several parameters have the same type. Moreover, marking a parameter as keyword-only allows us to change the order of parameters without breaking client code. Because of this, strongly consider marking optional parameters as keyword-only. In particular, optional hyperparameters of models should be keyword-only. + +!!! success "**DO** (library code):" + + ```py + class RandomForest: + def __init__(self, *, number_of_trees: int = 100) -> None: + ... + ``` + +!!! failure "**DON'T** (library code):" + + ```py + class RandomForest: + def __init__(self, number_of_trees: int = 100) -> None: + ... + ``` + ### Specify types of parameters and results Use [type hints](https://docs.python.org/3/library/typing.html) to describe the types of parameters and results of functions. This enables static type checking of client code.