saddle-points: update test suite to v1.3#621
Conversation
Emerentius
force-pushed
the
saddle_points
branch
from
ef33930 to
55d7d10
Compare
coriolinus
left a comment
coriolinus
left a comment
There was a problem hiding this comment.
I like it! Thanks for doing this work, @Emerentius. If you're willing, it would be helpful to open a PR adding this new case in the problem-specifications repo as well, so that other tracks will also be improved.
As with your other PR, I'm happy if another maintainer wants to merge immediatly. I'm on vacation, though, so won't be able to merge this myself before Monday the 27th.
|
I've copied this test from the python track actually. If I understand the system correctly, I think this exercise's test suite is still from version 1.1 of the problem specifications which have since changed to 1.3. |
|
That would be very helpful. Thanks! |
Emerentius
force-pushed
the
saddle_points
branch
from
55d7d10 to
d61e164
Compare
Emerentius
changed the title
|
I've updated the test suite. It was hard to see the matrices when all the rows where kept in one line so I split them apart. Sorry for the diff noise. It turns out that the track had several extensions of the standard tests. I replaced 2 with the overlapping ones from |
adds two new tests: - test_multiple_saddle_points_in_row - test_single_row_matrix some old, additional tests overlap with new tests replaced the following with standard tests: - test_non_square_matrix_wide - test_vector_matrix -> test_single_column_matrix kept one additional track-specific test: - test_non_square_matrix_high
Emerentius
force-pushed
the
saddle_points
branch
from
d61e164 to
a68d688
Compare
petertseng
left a comment
petertseng
left a comment
There was a problem hiding this comment.
It was hard to see the matrices when all the rows where kept in one line so I split them apart. Sorry for the diff noise.
It would be easier to review if the reformatting were in its own commit so it's easier to tell which changed lines were purely due to reformatting and which changed lines were due to going to 1.3
But since I can just review the entire new product against 1.3, I will just do it that way I suppose. It looks good.
petertseng
added
sync/readme
|
Great, thank you |
3 participants
This includes tests for a commonly unhandled case when two equally maximum elements
occur in a row that are both minima in their respective columns.
There is another test case already that has two equally maximum elements, but when a student checks only one element in a row and uses a max finder that always returns the right-most element of multiple equally large elements (like
Iterator::max), they happen to get the right result by luck.This also removes the existing implicit order dependence on the output of the student's code.