RFC: Fix handling of ambiguous cell types (#12)

[darwindarak]

Identifying contour directions using CW and CCW does not make any sense, especially when dealing with ambiguous cells. Instead of using a lookup-table, this new approach explicitly identifies how a contour crosses a cell using compass directions (e.g. a cell with a NW contour means that a contour line joins its north and west edges). Normal cells will have only one crossing, while ambiguous cells will have two crossings. As we process the list of cells, we only remove a cell when we have interpolated all of its contour crossings. This way, there shouldn't be any premature removal of cells. I'll add in more detailed documentation in the code.

Plotting the data from dcjones/Gadfly.jl#420 now gives

However, it currently fails the simpler test case given by @tlycken in #12

x = float([1:3]); y = copy(x); z = eye(3,3);
plot(z=z,x=x,y=y,Geom.contour)

gives

[tomasaschan]

@darwindarak Actually, I'm not sure that's a failure for z=eye(3,3). Intuitively it's easy to assume that the identity matrix should have only diagonal contour lines, but this assumption is incorrect.

Consider the corner exactly between the centers of two cells on the diagonal. Around that corner, there are two data points with z=1, and two with z=0, so the only reasonable assumption the interpolation can make is that a contour line crossing that corner corresponds to the value z=0.5. This means that all contour levels for z>0.5 will form squares around the ones, while all contour lines for 0 < z < 0.5 will form lines parallel to the diagonal - just what you see there.

The test failure I had for z=eye(3,3) also gave a no such key error, so if you've avoided that, you've probably fixed it. =)

Great work!

[tomasaschan]

By the way, some of the tests (for example, the one that currently fails) are very brittle, mainly because they're so implementation dependent. We should probably consider scrapping some of the tests that test the inner guts of the library, and just test that the output of exported functions is correct (i.e. that the correct contour levels are returned, and that the vertices are located in the right places etc).

[coveralls]

Coverage decreased (-1.33%) when pulling 437e05779b20bee28509b594e730a363b0eafe01 on dd/fix_saddle_point_issue into 016626d7a02bc6870ee72fbcb82cddc9a73303d1 on master.

[coveralls]

Coverage increased (+1.26%) when pulling 2ea4d9a16224b983ceb8d683248b94d3dbef354c on dd/fix_saddle_point_issue into 016626d7a02bc6870ee72fbcb82cddc9a73303d1 on master.

[darwindarak]

You're right, I didn't actually work out how the contours should look. So I think it's good to go!