Skip to content

Commit b715661

Browse files
committed
tests now show which method fails
1 parent 4290b9c commit b715661

File tree

1 file changed

+32
-28
lines changed

1 file changed

+32
-28
lines changed

tests/modules/test_training_feature_importances.py

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -83,22 +83,25 @@ def _get_available_models_by_type():
8383
return models_by_type
8484

8585

86-
def _generate_model_params():
87-
"""Generate (ml_type, model_name) param combos for pytest.
86+
def _generate_model_fi_params():
87+
"""Generate (ml_type, model_name, fi_method) param combos for pytest.
8888
89-
Each combo gets one test that runs ALL FI methods sequentially.
89+
Each combo gets its own test so that segfaults or crashes clearly
90+
identify which (model, FI method) combination failed.
9091
"""
9192
available_models = _get_available_models_by_type()
9293
params = []
9394
for ml_type, model_names in available_models.items():
9495
for model_name in model_names:
95-
params.append(
96-
pytest.param(
97-
ml_type,
98-
model_name,
99-
id=f"{ml_type.value}-{model_name}",
96+
for fi_method in FI_METHODS:
97+
params.append(
98+
pytest.param(
99+
ml_type,
100+
model_name,
101+
fi_method,
102+
id=f"{ml_type.value}-{model_name}-{fi_method}",
103+
)
100104
)
101-
)
102105
return params
103106

104107

@@ -236,19 +239,21 @@ def _run_fi_method(training: Training, method_name: str) -> list[str]:
236239

237240

238241
@pytest.mark.forked
239-
@pytest.mark.parametrize("ml_type,model_name", _generate_model_params())
240-
def test_feature_importance(ml_type, model_name):
241-
"""Test all FI methods for a single model in an isolated subprocess.
242+
@pytest.mark.parametrize("ml_type,model_name,fi_method", _generate_model_fi_params())
243+
def test_feature_importance(ml_type, model_name, fi_method):
244+
"""Test a single FI method for a single model in an isolated subprocess.
242245
243-
Each test runs in its own forked process (``@pytest.mark.forked``),
244-
providing complete isolation. This prevents:
246+
Each (model, FI method) combination runs in its own forked process
247+
(``@pytest.mark.forked``). This means:
245248
246-
- CatBoost C++ destructor segfaults during garbage collection
247-
- numba/llvmlite LLVM pass-manager crashes from accumulated JIT state
248-
- Memory accumulation across tests
249+
- A segfault clearly identifies the exact (model, method) that crashed
250+
- Complete process isolation prevents CatBoost GC segfaults,
251+
numba/llvmlite LLVM crashes, and memory accumulation
252+
- The trade-off is fitting the model once per test (slightly slower)
249253
250-
The model is fitted once, all FI methods run sequentially, and the
251-
entire process exits cleanly when the test completes.
254+
Test IDs look like::
255+
256+
test_feature_importance[binary-CatBoostClassifier-calculate_fi_featuresused_shap]
252257
"""
253258
warnings.filterwarnings("ignore")
254259

@@ -264,13 +269,12 @@ def test_feature_importance(ml_type, model_name):
264269
)
265270
training.fit()
266271

267-
for fi_method in FI_METHODS:
268-
fi_keys = _run_fi_method(training, fi_method)
272+
fi_keys = _run_fi_method(training, fi_method)
269273

270-
for key in fi_keys:
271-
fi_data = training.feature_importances.get(key)
272-
assert fi_data is not None, f"Feature importance key '{key}' not found after {fi_method}"
273-
# calculate_fi_internal legitimately returns empty for models without
274-
# built-in feature importances (e.g. GaussianProcess, SVM with non-linear kernel)
275-
if fi_method != "calculate_fi_internal":
276-
assert len(fi_data) > 0, f"Feature importance '{key}' is empty after {fi_method}"
274+
for key in fi_keys:
275+
fi_data = training.feature_importances.get(key)
276+
assert fi_data is not None, f"Feature importance key '{key}' not found after {fi_method}"
277+
# calculate_fi_internal legitimately returns empty for models without
278+
# built-in feature importances (e.g. GaussianProcess, SVM with non-linear kernel)
279+
if fi_method != "calculate_fi_internal":
280+
assert len(fi_data) > 0, f"Feature importance '{key}' is empty after {fi_method}"

0 commit comments

Comments
 (0)