Skip to content

Commit 5f018cc

Browse files
committed
cleanup tests and add explicit raise matches
1 parent 11e3b9b commit 5f018cc

26 files changed

Lines changed: 239 additions & 249 deletions

.github/copilot-instructions.md

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ Example: `FPS` (Farthest Point Sampling) exists as both `feature_selection.FPS`
2424

2525
## Development Workflows
2626

27+
Use 88 character line length limit for code and docstrings.
28+
2729
### Testing
2830
```bash
2931
# Run all tests with coverage
@@ -39,11 +41,27 @@ tox -e tests-dev
3941
Tests use pytest-style assertions and fixtures. Common patterns:
4042
- Use `@pytest.fixture` for test data setup
4143
- Use `assert` statements instead of `self.assertEqual()`
42-
- Use `pytest.raises()` for exception testing
44+
- Use `pytest.raises()` for exception testing always `match=` parameter. If match is too long that the `with` statement exceeds line length, define `match` variable before.
4345
- Use `pytest.warns()` for warning testing
4446
- Use `pytest.mark.parametrize` for parameterized tests
4547
- Tests often load datasets via `skmatter.datasets.load_*()`
4648

49+
**Exception Testing Style:**
50+
- Keep `with pytest.raises(...)` statement on one line (88 char limit)
51+
- For long match strings, define a `match` variable before the with statement:
52+
```python
53+
match = "Long error message that would exceed line length limit"
54+
with pytest.raises(ValueError, match=match):
55+
some_function()
56+
```
57+
- Use `re.escape()` when matching messages with special regex characters:
58+
```python
59+
import re
60+
match = f"Found array with shape={X.shape} ..."
61+
with pytest.raises(ValueError, match=re.escape(match)):
62+
selector.fit(X)
63+
```
64+
4765
### Linting & Formatting
4866
```bash
4967
# Check only (CI mode)

tests/test_clustering.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,5 +69,5 @@ def test_dimension_check(test_data):
6969
model = QuickShift(
7070
test_data["cuts"], metric_params={"cell_length": test_data["cell"]}
7171
)
72-
with pytest.raises(ValueError):
72+
with pytest.raises(ValueError, match="Dimension.*does not match"):
7373
model.fit(np.array([[2]]))

tests/test_datasets.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,8 @@ def test_load_csd_1000r_access_descr(csd):
9797
def test_load_dataset_without_pandas():
9898
"""Check if the correct exception occurs when pandas isn't present."""
9999
with mock.patch.dict("sys.modules", {"pandas": None}):
100-
with pytest.raises(ImportError) as cm:
101-
_ = load_who_dataset()
102-
assert str(cm.value) == "load_who_dataset requires pandas."
100+
with pytest.raises(ImportError, match="load_who_dataset requires pandas."):
101+
load_who_dataset()
103102

104103

105104
def test_dataset_size_and_shape(who_data):
@@ -115,7 +114,7 @@ def test_dataset_size_and_shape(who_data):
115114
def test_datapoint_value(who_data):
116115
"""Check if the value of a datapoint at a certain location is correct."""
117116
if who_data["has_pandas"]:
118-
assert np.allclose(
117+
np.testing.assert_allclose(
119118
who_data["who"]["data"]["SE.XPD.TOTL.GD.ZS"][1924],
120119
who_data["value"],
121120
rtol=1e-6,

tests/test_dch.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,21 +39,25 @@ def test_selected_idx_and_scores(test_data):
3939

4040
selector = DirectionalConvexHull()
4141
selector.fit(T, y)
42-
assert np.allclose(selector.selected_idx_, idx)
42+
np.testing.assert_allclose(selector.selected_idx_, idx)
4343

4444
# takes abs to avoid numerical noise changing the sign of PCA projections
4545
feature_residuals = np.abs(selector.score_feature_matrix(T))
4646
val = np.max(
4747
np.abs((feature_residuals_100 - feature_residuals[100]) / feature_residuals_100)
4848
)
49-
assert np.allclose(feature_residuals_100, feature_residuals[100], rtol=1e-6), (
50-
f"Maximum relative error 1e-6 < {val}"
49+
(
50+
np.testing.assert_allclose(
51+
feature_residuals_100, feature_residuals[100], rtol=1e-6
52+
),
53+
(f"Maximum relative error 1e-6 < {val}"),
5154
)
5255

5356
y_distances = selector.score_samples(T, y)
5457
val = np.max(np.abs((y_distance_100 - y_distances[100]) / y_distance_100))
55-
assert np.allclose(y_distance_100, y_distances[100], rtol=1e-6), (
56-
f"Maximum relative error 1e-6 < {val}"
58+
(
59+
np.testing.assert_allclose(y_distance_100, y_distances[100], rtol=1e-6),
60+
(f"Maximum relative error 1e-6 < {val}"),
5761
)
5862

5963

@@ -94,7 +98,7 @@ def test_residual_features_without_fit(test_data):
9498
"""
9599
T = test_data["T"]
96100
selector = DirectionalConvexHull()
97-
with pytest.raises(NotFittedError):
101+
with pytest.raises(NotFittedError, match="instance is not fitted"):
98102
selector.score_feature_matrix(T)
99103

100104

@@ -109,11 +113,11 @@ def test_residual_features_ndim(test_data):
109113

110114
selector = DirectionalConvexHull()
111115
selector.fit(T, y)
112-
with pytest.raises(ValueError) as cm:
113-
selector.score_feature_matrix(X)
114-
assert str(cm.value) == (
116+
match = (
115117
"X has 10 features, but DirectionalConvexHull is expecting 4 features as input."
116118
)
119+
with pytest.raises(ValueError, match=match):
120+
selector.score_feature_matrix(X)
117121

118122

119123
def test_negative_score(test_data):

tests/test_feature_pcov_cur.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def test_known(X_y_idx):
1818
selector = PCovCUR(n_to_select=9)
1919
selector.fit(X, y)
2020

21-
assert np.allclose(selector.selected_idx_, idx)
21+
np.testing.assert_allclose(selector.selected_idx_, idx)
2222

2323

2424
def test_restart(X_y_idx):
@@ -39,4 +39,4 @@ def test_non_it(X_y_idx):
3939
idx = [2, 8, 3, 6, 7, 9, 1, 0, 5]
4040
selector = PCovCUR(n_to_select=9, recompute_every=0)
4141
selector.fit(X, y)
42-
assert np.allclose(selector.selected_idx_, idx)
42+
np.testing.assert_allclose(selector.selected_idx_, idx)

tests/test_feature_pcov_fps.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,6 @@ def test_no_mixing_1(X_y_idx):
2929
"""Check that the model throws an error when mixing = 1.0."""
3030
X, y, _ = X_y_idx
3131
selector = PCovFPS(n_to_select=1, mixing=1.0)
32-
with pytest.raises(ValueError) as cm:
32+
match = "Mixing = 1.0 corresponds to traditional FPS. Please use the FPS class."
33+
with pytest.raises(ValueError, match=match):
3334
selector.fit(X, y=y)
34-
assert (
35-
str(cm.value)
36-
== "Mixing = 1.0 corresponds to traditional FPS. Please use the FPS class."
37-
)

tests/test_feature_simple_cur.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ def X():
1414

1515
def test_bad_transform(X):
1616
selector = CUR(n_to_select=2)
17-
with pytest.raises(exceptions.NotFittedError):
18-
_ = selector.transform(X)
17+
with pytest.raises(exceptions.NotFittedError, match="instance is not fitted"):
18+
selector.transform(X)
1919

2020

2121
def test_restart(X):
@@ -41,7 +41,7 @@ def test_non_it(X):
4141
selector = CUR(n_to_select=X.shape[-1] - 1, recompute_every=0)
4242
selector.fit(X)
4343

44-
assert np.allclose(selector.selected_idx_, ref_idx)
44+
np.testing.assert_allclose(selector.selected_idx_, ref_idx)
4545

4646

4747
def test_unique_selected_idx_zero_score():

tests/test_feature_simple_fps.py

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,22 +50,21 @@ def test_initialize(X_and_idx):
5050
for i in range(4):
5151
assert selector.selected_idx_[i] == idx[i]
5252

53+
match = "Invalid value of the initialize parameter"
54+
5355
initialize = np.array([1, 5, 3, 0.25])
54-
with pytest.raises(ValueError) as cm:
56+
with pytest.raises(ValueError, match=match):
5557
selector = FPS(n_to_select=len(idx) - 1, initialize=initialize)
5658
selector.fit(X)
57-
assert str(cm.value) == "Invalid value of the initialize parameter"
5859

5960
initialize = np.array([[1, 5, 3], [2, 4, 6]])
60-
with pytest.raises(ValueError) as cm:
61+
with pytest.raises(ValueError, match=match):
6162
selector = FPS(n_to_select=len(idx) - 1, initialize=initialize)
6263
selector.fit(X)
63-
assert str(cm.value) == "Invalid value of the initialize parameter"
6464

65-
with pytest.raises(ValueError) as cm:
65+
with pytest.raises(ValueError, match=match):
6666
selector = FPS(n_to_select=1, initialize="bad")
6767
selector.fit(X)
68-
assert str(cm.value) == "Invalid value of the initialize parameter"
6968

7069

7170
def test_get_distances(X_and_idx):
@@ -80,7 +79,7 @@ def test_get_distances(X_and_idx):
8079

8180
with pytest.raises(NotFittedError):
8281
selector = FPS(n_to_select=7)
83-
_ = selector.get_select_distance()
82+
selector.get_select_distance()
8483

8584

8685
def test_unique_selected_idx_zero_score():

tests/test_greedy_selector.py

Lines changed: 33 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import re
2+
13
import numpy as np
24
import pytest
35
from sklearn.datasets import load_diabetes as get_dataset
@@ -30,10 +32,9 @@ def X():
3032

3133

3234
def test_bad_type(X):
33-
with pytest.raises(
34-
ValueError, match="Only feature and sample selection supported."
35-
):
36-
_ = GreedyTester(selection_type="bad").fit(X)
35+
match = "Only feature and sample selection supported."
36+
with pytest.raises(ValueError, match=match):
37+
GreedyTester(selection_type="bad").fit(X)
3738

3839

3940
def test_score_threshold(X):
@@ -46,48 +47,38 @@ def test_score_threshold(X):
4647

4748

4849
def test_score_threshold_and_full(X):
49-
with pytest.raises(ValueError) as cm:
50-
_ = GreedyTester(score_threshold=20, full=True, n_to_select=12).fit(X)
51-
assert str(cm.value) == "You cannot specify both `score_threshold` and `full=True`."
50+
match = "You cannot specify both `score_threshold` and `full=True`."
51+
with pytest.raises(ValueError, match=match):
52+
GreedyTester(score_threshold=20, full=True, n_to_select=12).fit(X)
5253

5354

5455
def test_bad_score_threshold_type(X):
55-
with pytest.raises(ValueError) as cm:
56-
_ = GreedyTester(score_threshold_type="bad").fit(X)
57-
assert (
58-
str(cm.value)
59-
== "invalid score_threshold_type, expected one of 'relative' or 'absolute'"
60-
)
56+
match = "invalid score_threshold_type, expected one of 'relative' or 'absolute'"
57+
with pytest.raises(ValueError, match=match):
58+
GreedyTester(score_threshold_type="bad").fit(X)
6159

6260

6361
def test_bad_warm_start(X):
6462
selector = GreedyTester()
65-
with pytest.raises(
66-
ValueError,
67-
match=(
68-
"Cannot fit with warm_start=True without having been previously initialized"
69-
),
70-
):
63+
match = "Cannot fit with warm_start=True without having been previously initialized"
64+
with pytest.raises(ValueError, match=match):
7165
selector.fit(X, warm_start=True)
7266

7367

7468
def test_bad_y(X):
7569
_, Y = get_dataset(return_X_y=True)
7670
Y = Y[:2]
7771
selector = GreedyTester(n_to_select=2)
78-
with pytest.raises(ValueError):
72+
with pytest.raises(ValueError, match="inconsistent numbers of samples"):
7973
selector.fit(X=X, y=Y)
8074

8175

8276
def test_bad_transform(X):
8377
selector = GreedyTester(n_to_select=2)
8478
selector.fit(X)
85-
with pytest.raises(ValueError) as cm:
86-
_ = selector.transform(X[:, :3])
87-
assert (
88-
str(cm.value)
89-
== "X has 3 features, but GreedyTester is expecting 10 features as input."
90-
)
79+
match = "X has 3 features, but GreedyTester is expecting 10 features as input."
80+
with pytest.raises(ValueError, match=match):
81+
selector.transform(X[:, :3])
9182

9283

9384
def test_no_nfeatures(X):
@@ -105,27 +96,26 @@ def test_decimal_nfeatures(X):
10596
@pytest.mark.parametrize("nf", [1.2, "1", 20])
10697
def test_bad_nfeatures(X, nf):
10798
selector = GreedyTester(n_to_select=nf)
108-
with pytest.raises(ValueError) as cm:
109-
selector.fit(X)
110-
assert str(cm.value) == (
111-
"n_to_select must be either None, an integer in "
112-
"[1, n_features] representing the absolute number "
113-
"of features, or a float in (0, 1] representing a "
114-
f"percentage of features to select. Got {nf} "
115-
f"features and an input with {X.shape[1]} feature."
99+
expected_msg = (
100+
"n_to_select must be either None, an integer in [1, n_features] representing "
101+
"the absolute number of features, or a float in (0, 1] representing a "
102+
f"percentage of features to select. Got {nf} features and an input with "
103+
f"{X.shape[1]} feature."
116104
)
105+
with pytest.raises(ValueError, match=re.escape(expected_msg)):
106+
selector.fit(X)
117107

118108

119109
def test_not_fitted():
120-
with pytest.raises(NotFittedError):
110+
with pytest.raises(NotFittedError, match="instance is not fitted"):
121111
selector = GreedyTester()
122-
_ = selector._get_support_mask()
112+
selector._get_support_mask()
123113

124114

125115
def test_fitted(X):
126116
selector = GreedyTester()
127117
selector.fit(X)
128-
_ = selector._get_support_mask()
118+
selector._get_support_mask()
129119

130120
Xr = selector.transform(X)
131121
assert Xr.shape[1] == X.shape[1] // 2
@@ -135,18 +125,17 @@ def test_size_input():
135125
X = np.array([1, 2, 3, 4, 5]).reshape(-1, 1)
136126
selector_sample = GreedyTester(selection_type="sample")
137127
selector_feature = GreedyTester(selection_type="feature")
138-
with pytest.raises(ValueError) as cm:
139-
selector_feature.fit(X)
140-
assert str(cm.value) == (
128+
expected_msg_feature = (
141129
f"Found array with 1 feature(s) (shape={X.shape}) while a minimum of 2 is "
142130
"required by GreedyTester."
143131
)
132+
with pytest.raises(ValueError, match=re.escape(expected_msg_feature)):
133+
selector_feature.fit(X)
144134

145135
X = X.reshape(1, -1)
146-
147-
with pytest.raises(ValueError) as cm:
148-
selector_sample.fit(X)
149-
assert str(cm.value) == (
136+
expected_msg_sample = (
150137
f"Found array with 1 sample(s) (shape={X.shape}) while a minimum of 2 is "
151138
"required by GreedyTester."
152139
)
140+
with pytest.raises(ValueError, match=re.escape(expected_msg_sample)):
141+
selector_sample.fit(X)

0 commit comments

Comments
 (0)