[tests] replace pytest.parametrize #4377
Conversation
StrikerRUS
requested review from
chivee,
henry0312,
shiyu1994 and
wxchan
as code owners
| trainset = generate_trainset_for_monotone_constraints_tests( | ||
| test_with_categorical_variable | ||
| ) | ||
| for test_with_interaction_constraints in [True, False]: |
There was a problem hiding this comment.
I'm ok with this change, but could you update the assert on line 1334 with an error message that will tell us which case it failed on if it fails?
error_msg = f"Model not correctly constrained (test_with_interaction_constraints={test_with_interaction_constraints})"
assert is_correctly_constrained(
constrained_model, test_with_categorical_variable
), error_msgWithout that, that particular test could fail and we wouldn't know which case it failed on just from the logs.
I think it would also be useful to include the value of monotone_constraints_method in that message too (since it is similarly from an inner for loop), but that's not directly related to this PR so up to you if you want to do that at the same time or separately.
There was a problem hiding this comment.
Good point! Done in 4445482.
| predict(x3_negatively_correlated_with_y.reshape(-1, 1)) | ||
| unconstrained_model_predictions = unconstrained_model.predict( | ||
| x3_negatively_correlated_with_y.reshape(-1, 1) | ||
| ) |
There was a problem hiding this comment.
I like this formatting a lot more, thank you
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Avoid unnecessary trainset generation.