From 8fc975ce10a116e9afd217890a1d701120daffea Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Mon, 13 Sep 2021 14:53:15 +0200 Subject: [PATCH 01/10] Add venv to gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 8321c7d2..16917890 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,5 @@ htmlcov/ .cache/ .pytest_cache/ doc/auto_examples/* -doc/generated/* \ No newline at end of file +doc/generated/* +venv/ \ No newline at end of file From 40bbce803990d8008635b90e8e6309b02d9306b8 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Mon, 13 Sep 2021 16:16:51 +0200 Subject: [PATCH 02/10] Check if threshold is a real value --- metric_learn/base_metric.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 721d7ba0..fca2fca9 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -410,6 +410,10 @@ def set_threshold(self, threshold): """ check_is_fitted(self, 'preprocessor_') + if threshold is None or not isinstance(threshold, (int, float)): + raise ValueError('Parameter threshold must be a real number. ' + 'Got {} instead.'.format(type(threshold))) + self.threshold_ = threshold return self From 60dc0d42d8311b9d1220df4e747537d37a32a2f0 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Fri, 17 Sep 2021 10:21:38 +0200 Subject: [PATCH 03/10] Simplified threshold type-check --- metric_learn/base_metric.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index fca2fca9..1d22439a 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -410,11 +410,7 @@ def set_threshold(self, threshold): """ check_is_fitted(self, 'preprocessor_') - if threshold is None or not isinstance(threshold, (int, float)): - raise ValueError('Parameter threshold must be a real number. ' - 'Got {} instead.'.format(type(threshold))) - - self.threshold_ = threshold + self.threshold_ = float(threshold) # Will raise ValueError if input was not a number return self def calibrate_threshold(self, pairs_valid, y_valid, strategy='accuracy', From a92d2c950e1add21d39b1908f29d91f008b3832d Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Fri, 17 Sep 2021 10:41:22 +0200 Subject: [PATCH 04/10] Follow linter rules --- metric_learn/base_metric.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 1d22439a..180f9a4d 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -409,8 +409,9 @@ def set_threshold(self, threshold): The pairs classifier with the new threshold set. """ check_is_fitted(self, 'preprocessor_') - - self.threshold_ = float(threshold) # Will raise ValueError if input was not a number + + # Will raise ValueError if input was not a number + self.threshold_ = float(threshold) return self def calibrate_threshold(self, pairs_valid, y_valid, strategy='accuracy', From ddce7dbef3343b42956663c2f9da916988ac136f Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Tue, 21 Sep 2021 15:44:33 +0200 Subject: [PATCH 05/10] Fix last linter error --- metric_learn/base_metric.py | 1 - 1 file changed, 1 deletion(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 180f9a4d..1526d1e8 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -409,7 +409,6 @@ def set_threshold(self, threshold): The pairs classifier with the new threshold set. """ check_is_fitted(self, 'preprocessor_') - # Will raise ValueError if input was not a number self.threshold_ = float(threshold) return self From ac4584ebff118573911e599c491eff4baf3b36f0 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Tue, 21 Sep 2021 16:06:21 +0200 Subject: [PATCH 06/10] Add test to check correct behaviour. Sacrified simplicity for the bool case. --- metric_learn/base_metric.py | 4 +++- test/test_pairs_classifiers.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 1526d1e8..241e635b 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -409,7 +409,9 @@ def set_threshold(self, threshold): The pairs classifier with the new threshold set. """ check_is_fitted(self, 'preprocessor_') - # Will raise ValueError if input was not a number + if threshold is isinstance(threshold, (bool)): + raise ValueError('Parameter threshold must be a real number. ' + 'Got {} instead.'.format(type(threshold))) self.threshold_ = float(threshold) return self diff --git a/test/test_pairs_classifiers.py b/test/test_pairs_classifiers.py index 824bb622..f2388b14 100644 --- a/test/test_pairs_classifiers.py +++ b/test/test_pairs_classifiers.py @@ -178,6 +178,36 @@ def test_set_threshold(): assert identity_pairs_classifier.threshold_ == 0.5 +def test_set_wrong_type_threshold(): + # test that set_threshold indeed sets the threshold + # and cannot accept nothing but float or integers + identity_pairs_classifier = IdentityPairsClassifier() + pairs = np.array([[[0.], [1.]], [[1.], [3.]], [[2.], [5.]], [[3.], [7.]]]) + y = np.array([1, 1, -1, -1]) + identity_pairs_classifier.fit(pairs, y) + with pytest.raises(ValueError): + identity_pairs_classifier.set_threshold("ABC") # String + with pytest.raises(ValueError): + identity_pairs_classifier.set_threshold(True) # Bool + with pytest.raises(TypeError): + identity_pairs_classifier.set_threshold(None) # None + with pytest.raises(TypeError): + identity_pairs_classifier.set_threshold([1, 2, 3]) # List + with pytest.raises(TypeError): + identity_pairs_classifier.set_threshold({'key': None}) # Dict + with pytest.raises(TypeError): + identity_pairs_classifier.set_threshold((1, 2)) # Tuple + with pytest.raises(TypeError): + identity_pairs_classifier.set_threshold(set()) # Set + with pytest.raises(TypeError): + identity_pairs_classifier.set_threshold(pairs) # np.array + identity_pairs_classifier.set_threshold(1) # Integer + identity_pairs_classifier.set_threshold(0.1) # Float + identity_pairs_classifier.set_threshold(np.array([0.5])) # 1D np.array + identity_pairs_classifier.set_threshold(np.array([[[0.5]]])) # 1D* np.array + assert identity_pairs_classifier.threshold_ == 0.5 + + def test_f_beta_1_is_f_1(): # test that putting beta to 1 indeed finds the best threshold to optimize # the f1_score From 166228fc0b98844375f769bb4c4521d75713e472 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Thu, 28 Oct 2021 11:42:12 +0200 Subject: [PATCH 07/10] Update test. Stick to custom message. It's bool permissive --- metric_learn/base_metric.py | 8 +++--- test/test_pairs_classifiers.py | 51 +++++++++++++++------------------- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index 27c1243d..a7baa1b1 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -569,10 +569,10 @@ def set_threshold(self, threshold): The pairs classifier with the new threshold set. """ check_is_fitted(self, 'preprocessor_') - if threshold is isinstance(threshold, (bool)): - raise ValueError('Parameter threshold must be a real number. ' - 'Got {} instead.'.format(type(threshold))) - self.threshold_ = float(threshold) + if not isinstance(threshold, (int, float)): + raise ValueError('Parameter threshold must be a real number. ' + 'Got {} instead.'.format(type(threshold))) + self.threshold_ = float(threshold) # int -> float return self def calibrate_threshold(self, pairs_valid, y_valid, strategy='accuracy', diff --git a/test/test_pairs_classifiers.py b/test/test_pairs_classifiers.py index ecb52f24..ffd728f6 100644 --- a/test/test_pairs_classifiers.py +++ b/test/test_pairs_classifiers.py @@ -180,34 +180,29 @@ def test_set_threshold(): assert identity_pairs_classifier.threshold_ == 0.5 -def test_set_wrong_type_threshold(): - # test that set_threshold indeed sets the threshold - # and cannot accept nothing but float or integers - identity_pairs_classifier = IdentityPairsClassifier() - pairs = np.array([[[0.], [1.]], [[1.], [3.]], [[2.], [5.]], [[3.], [7.]]]) - y = np.array([1, 1, -1, -1]) - identity_pairs_classifier.fit(pairs, y) - with pytest.raises(ValueError): - identity_pairs_classifier.set_threshold("ABC") # String - with pytest.raises(ValueError): - identity_pairs_classifier.set_threshold(True) # Bool - with pytest.raises(TypeError): - identity_pairs_classifier.set_threshold(None) # None - with pytest.raises(TypeError): - identity_pairs_classifier.set_threshold([1, 2, 3]) # List - with pytest.raises(TypeError): - identity_pairs_classifier.set_threshold({'key': None}) # Dict - with pytest.raises(TypeError): - identity_pairs_classifier.set_threshold((1, 2)) # Tuple - with pytest.raises(TypeError): - identity_pairs_classifier.set_threshold(set()) # Set - with pytest.raises(TypeError): - identity_pairs_classifier.set_threshold(pairs) # np.array - identity_pairs_classifier.set_threshold(1) # Integer - identity_pairs_classifier.set_threshold(0.1) # Float - identity_pairs_classifier.set_threshold(np.array([0.5])) # 1D np.array - identity_pairs_classifier.set_threshold(np.array([[[0.5]]])) # 1D* np.array - assert identity_pairs_classifier.threshold_ == 0.5 +@pytest.mark.parametrize('value', ["ABC", None, [1, 2, 3], {'key': None}, + (1, 2), set(), + np.array([[[0.], [1.]], [[1.], [3.]]]), + np.array([0.5]), np.array([[[0.5]]])]) +def test_set_wrong_type_threshold(value): + """ + Test that `set_threshold` indeed sets the threshold + and cannot accept nothing but float or integers, but + being permissive with boolean True=1.0 and False=0.0 + """ + model = IdentityPairsClassifier() + model.fit(np.array([[[0.], [1.]]]), np.array([1])) + msg = ('Parameter threshold must be a real number. ' + 'Got {} instead.'.format(type(value))) + + with pytest.raises(ValueError) as e: # String + model.set_threshold(value) + assert str(e.value).startswith(msg) + + model.set_threshold(1) # Integer + assert model.threshold_ == 1.0 + model.set_threshold(0.1) # Float + assert model.threshold_ == 0.1 def test_f_beta_1_is_f_1(): From 47c21ac0874433e47aeead71cc4edf74027b48df Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Thu, 28 Oct 2021 18:10:24 +0200 Subject: [PATCH 08/10] Explicit boolean permissive case in test --- test/test_pairs_classifiers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/test_pairs_classifiers.py b/test/test_pairs_classifiers.py index ffd728f6..33aa7668 100644 --- a/test/test_pairs_classifiers.py +++ b/test/test_pairs_classifiers.py @@ -203,6 +203,10 @@ def test_set_wrong_type_threshold(value): assert model.threshold_ == 1.0 model.set_threshold(0.1) # Float assert model.threshold_ == 0.1 + model.set_threshold(True) # Boolean permissive + assert model.threshold_ == 1.0 + model.set_threshold(False) # Boolean permissive + assert model.threshold_ == 0.0 def test_f_beta_1_is_f_1(): From 81c7f73ffd8786b65f1884ab2484882dbc0c4c26 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Thu, 28 Oct 2021 18:17:02 +0200 Subject: [PATCH 09/10] Changed isinstance for custom ValueError message --- metric_learn/base_metric.py | 5 +++-- test/test_pairs_classifiers.py | 12 +----------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index a7baa1b1..dff32eeb 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -569,10 +569,11 @@ def set_threshold(self, threshold): The pairs classifier with the new threshold set. """ check_is_fitted(self, 'preprocessor_') - if not isinstance(threshold, (int, float)): + try: + self.threshold_ = float(threshold) + except Exception: raise ValueError('Parameter threshold must be a real number. ' 'Got {} instead.'.format(type(threshold))) - self.threshold_ = float(threshold) # int -> float return self def calibrate_threshold(self, pairs_valid, y_valid, strategy='accuracy', diff --git a/test/test_pairs_classifiers.py b/test/test_pairs_classifiers.py index 33aa7668..6a725f23 100644 --- a/test/test_pairs_classifiers.py +++ b/test/test_pairs_classifiers.py @@ -182,8 +182,7 @@ def test_set_threshold(): @pytest.mark.parametrize('value', ["ABC", None, [1, 2, 3], {'key': None}, (1, 2), set(), - np.array([[[0.], [1.]], [[1.], [3.]]]), - np.array([0.5]), np.array([[[0.5]]])]) + np.array([[[0.], [1.]], [[1.], [3.]]])]) def test_set_wrong_type_threshold(value): """ Test that `set_threshold` indeed sets the threshold @@ -199,15 +198,6 @@ def test_set_wrong_type_threshold(value): model.set_threshold(value) assert str(e.value).startswith(msg) - model.set_threshold(1) # Integer - assert model.threshold_ == 1.0 - model.set_threshold(0.1) # Float - assert model.threshold_ == 0.1 - model.set_threshold(True) # Boolean permissive - assert model.threshold_ == 1.0 - model.set_threshold(False) # Boolean permissive - assert model.threshold_ == 0.0 - def test_f_beta_1_is_f_1(): # test that putting beta to 1 indeed finds the best threshold to optimize From 796ba93912e75a1477fdd4dd8fbb0f4732a0def9 Mon Sep 17 00:00:00 2001 From: mvargas33 Date: Tue, 2 Nov 2021 10:51:46 +0100 Subject: [PATCH 10/10] TypeError for most input. ValueError for String case. --- metric_learn/base_metric.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/metric_learn/base_metric.py b/metric_learn/base_metric.py index dff32eeb..9064c100 100644 --- a/metric_learn/base_metric.py +++ b/metric_learn/base_metric.py @@ -571,7 +571,10 @@ def set_threshold(self, threshold): check_is_fitted(self, 'preprocessor_') try: self.threshold_ = float(threshold) - except Exception: + except TypeError: + raise ValueError('Parameter threshold must be a real number. ' + 'Got {} instead.'.format(type(threshold))) + except ValueError: raise ValueError('Parameter threshold must be a real number. ' 'Got {} instead.'.format(type(threshold))) return self