Added the StdDequeIterator and its testset #433
Conversation
src/StdLib.jl
Outdated
| Base.iterate(d::StdDeque, state::StdIterator) = (state != iteratorend(d)) ? _deque_iteration_tuple(d, iterator_next(state)) : nothing | ||
| #TODO:remove the iterator_value method from the cxx part, since it is not needed | ||
| end # module | ||
| Base.size(v::StdDeque) = (Int(cppsize(v)),) |
There was a problem hiding this comment.
Thank you for your patch!
However, I would recommend to not mix actual changes with changes that rename arguments or reorder code, as it makes it needlessly difficult to review your changes.
Here, you changed all d::StdDeque to v::StdDeque which has no obvious benefit in itself. Perhaps you prefer it, but then you should put it into a separate commit, or even separate PR. The same applies to reordering the methods. Together the result is that it is very difficult to see if you changed anything in the implementation of these methods, and what exactly.
I would really prefer if this part of the change was undone, but that's just me, @barche may disagree :-)
There was a problem hiding this comment.
Thanks for the advice, I will stick to it in the future.
I renamed the parameters back to the d and the ordering was my fault as it was not aligned with the ordering of the C++ part of the component.
| Base.resize!(v::StdDeque, n::Integer) = resize(v, n) | ||
| Base.empty!(v::StdDeque) = clear(v) | ||
|
|
||
| Base.:(==)(a::StdDequeIterator, b::StdDequeIterator) = iterator_is_equal(a,b) |
There was a problem hiding this comment.
Tests fail because StdDequeIterator is not defined.
Perhaps this PR is supposed to be used together with a changed libcxxwrap-julia ? Perhaps with JuliaInterop/libcxxwrap-julia#133 ? (It would be a good idea to point out something like this in the PR description)
| Base.size(v::StdQueue) = (Int(cppsize(v)),) | ||
| Base.push!(v::StdQueue, x) = push_back(v, x) | ||
| Base.first(v::StdQueue) = front(v) | ||
| Base.pop!(v::StdQueue) = pop_front(v) |
There was a problem hiding this comment.
This code seems to be unrelated to StdDeque? Perhaps this PR needs to be rebased on the latest CxxWrap master?
There was a problem hiding this comment.
I can't handle this nicely in one commit and keep this out of the changes as the testjll-stditerator branch itself is far behind the master.
|
This was integrated with #448 |
I've removed the
!s from the end of the mutable functions in both PRs and added some tests for the iterator. The C++ part of it is in this PR JuliaInterop/libcxxwrap-julia#133