Implement str iterator support #1171
Conversation
coolreader18
left a comment
coolreader18
left a comment
There was a problem hiding this comment.
This looks good, thank you for contributing to the project! I've made a comment on your changes and the CI is failing due to a linting error, but fix that up and this is good to merge.
| #[pymethod(name = "__iter__")] | ||
| fn iter(zelf: PyRef<Self>, _vm: &VirtualMachine) -> PyStringIterator { | ||
| PyStringIterator { | ||
| position: Cell::new(0), |
There was a problem hiding this comment.
I believe you're missing the __reversed__ method somewhere here?
There was a problem hiding this comment.
Actually, I checked it using CPython (3.7.3):
>>> hasattr(str, "__reversed__")
False
Should I implement __reversed__ despite this fact or leave it unchanged?
There was a problem hiding this comment.
How is PyStringReverseIterator used then?
There was a problem hiding this comment.
Within reverse:
string = "123456789"
reversed_iter = reversed(string)
There was a problem hiding this comment.
Right, but nowhere in your code do you construct a PyStringReverseIterator, so there's no way to actually get one in Python code.
There was a problem hiding this comment.
Ok! I'll fix it in a moment.
|
Thanks for your feedback :) I'll deliver corrections today. |
PySliceableSequence trait methods require Range<usize> as arguments
|
I corrected lint warning and added |
2 participants
Hello,
I added iteration support for
str.