Conversation
mfeurer
left a comment
mfeurer
left a comment
There was a problem hiding this comment.
This looks good so far, I have a few comments and suggestions. I added them only for one of the classes, but please apply them to all of the classes.
test/test_forbidden.py
Outdated
| assert forb1.is_forbidden_vector(np.array([1.0, np.nan])) | ||
|
|
||
|
|
||
| def test_forbidden_greater_than_clause(): |
There was a problem hiding this comment.
This test is too long. Could you please split it up in shorter tests, one for each type that the clause is tested on?
There was a problem hiding this comment.
This is already separated, one for greater_than and one for greater_than_equals. Is there another split I can do to make the tests smaller?
There was a problem hiding this comment.
Separate the tests into the cross-product of clause type and hyperparameter type?
There was a problem hiding this comment.
Parametrised the test now with hyper parameter and values to make it a lot shorter
There was a problem hiding this comment.
That looks good. If you now could also parametrize the other clauses and relations, we can go ahead and merge this one.
2 participants
Closes #402