Fixes for 0.7 #10
Fixes for 0.7 #10
Conversation
| @@ -3,8 +3,7 @@ os: | |||
| - linux | |||
| - osx | |||
| julia: | |||
| - 0.4 | |||
| - 0.5 | |||
| - 0.7 | |||
| - nightly | |||
There was a problem hiding this comment.
the only real difference with #11 is that i allowed failures on nightly to make hexagons like gadfly and compose.
| @@ -1,2 +1,2 @@ | |||
| julia 0.4 | |||
| julia 0.7 | |||
| Compat 0.17 | |||
There was a problem hiding this comment.
are their any remaining calls to @compat ? if not can we remove this requirement?
|
|
||
| function next(it::HexagonNeighborIterator, state::Int) | ||
| function Base.iterate(it::HexagonNeighborIterator, state=1) |
There was a problem hiding this comment.
here and elsewhere, must one qualify iterate with Base. ?
| new((@compat Float64(x)), (@compat Float64(y)), | ||
| (@compat Float64(xsize)), (@compat Float64(ysize))) | ||
| new((Float64(x)), (Float64(y)), | ||
| (Float64(xsize)), (Float64(ysize))) |
There was a problem hiding this comment.
here and elsewhere, there are extra parens that could be removed
|
|
||
| # The state of a HexagonSpiralIterator consists of | ||
| # 1. an Int, the index of the current ring | ||
| # 2. a HexagonRingIterator and its state to keep track of the current position | ||
| # in the ring. | ||
| function next(it::HexagonSpiralIterator, state::HexagonSpiralIteratorState) | ||
| function Base.iterate(it::HexagonSpiralIterator, | ||
| state::HexagonSpiralIteratorState=_start(it)) |
There was a problem hiding this comment.
are their pros and cons to calling _start in the function signature, versus creating a second iterate method with a single input argument as i did in #11 ? the latter is easier to read to me, but i'm not familiar enough with julia internals to know if there's a performance difference.
|
No worries @bjarthur! I don't use Hexagons for anything other than Gadfly. Also a good chance to learn the new iteration protocol myself 😄. I will go over your comments in the next few days |
|
great! we're hoping to release a beta gadfly for field testing in the next few days too. this PR is critical to that plan. |
|
Now that the monster Compose PR is merged (GiovineItalia/Compose.jl#282). This PR is high priority for getting Gadfly working. @rsrock do you have any time to look at this in the next couple of days? @bjarthur might be worth re-opening your PR if we don't hear back from Ron. I'm fine with either iteration implementation. |
No description provided.