Skip to content

[RFC] Add eachrsplit iterator#51646

Merged
vtjnash merged 1 commit intoJuliaLang:masterfrom
jakobnissen:eachrsplit
Oct 11, 2023
Merged

[RFC] Add eachrsplit iterator#51646
vtjnash merged 1 commit intoJuliaLang:masterfrom
jakobnissen:eachrsplit

Conversation

@jakobnissen
Copy link
Member

@jakobnissen jakobnissen commented Oct 9, 2023

Unlike rsplit, this iterator returns split substrings right to left, but other- wise it behaves just like eachsplit.
This design has been chosen to avoid either a costly double traversal of the input string, or needing a stack to store the strings. Both of these workarounds would lessen the appeal compared to simply using rsplit.

Closes #45385

Request for comments

  • Is it acceptable that it returns the substrings in reverse order compared to rsplit? I believe this is unfortunately necessary.

@jakobnissen jakobnissen added needs compat annotation Add !!! compat "Julia x.y" to the docstring feature Indicates new feature / enhancement requests strings "Strings!" iteration Involves iteration or the iteration protocol labels Oct 10, 2023
@jakobnissen jakobnissen removed the needs compat annotation Add !!! compat "Julia x.y" to the docstring label Oct 10, 2023
@jakobnissen jakobnissen marked this pull request as ready for review October 10, 2023 08:47
@jakobnissen jakobnissen force-pushed the eachrsplit branch 2 times, most recently from 7967578 to 0ea52a5 Compare October 10, 2023 09:30
@jakobnissen jakobnissen requested a review from stevengj October 10, 2023 11:56
Unlike rsplit, this iterator returns split substrings right to left, but other-
wise it behaves just like eachsplit.
This design has been chosen to avoid either a costly double traversal of the
input string, or needing a stack to store the strings. Both of these workarounds
would lessen the appeal compared to simply using rsplit.
@jakobnissen
Copy link
Member Author

Squashed, otherwise unchanged. I think the ASAN build failure is unrelated to this PR.

@jakobnissen jakobnissen added the merge me PR is reviewed. Merge when all tests are passing label Oct 11, 2023
@vtjnash vtjnash merged commit be1702e into JuliaLang:master Oct 11, 2023
@oscardssmith oscardssmith removed the merge me PR is reviewed. Merge when all tests are passing label Oct 11, 2023
@jakobnissen jakobnissen deleted the eachrsplit branch October 11, 2023 20:28
@stevengj
Copy link
Member

stevengj commented Oct 8, 2024

Is there a reason why this wasn't implemented as an Iterators.Reverse{SplitIterator}, so that you could just do Iterators.reverse(eachsplit(...)), and instead introduced a completely new iterator type and API?

@vtjnash
Copy link
Member

vtjnash commented Oct 8, 2024

IIRC, this isn't just Iterators.reverse(eachsplit) because of the limit parameter

@stevengj
Copy link
Member

stevengj commented Oct 8, 2024

I see. So we could implement something like:

function Iterators.reverse(itr::Union{SplitIterator,RSplitIterator})
    itr.limit <= 0 || throw(ArgumentError("reverse does not support split iterators with limit = $(itr.limit) > 0"))
    return (itr isa SplitIterator ? RSplitIterator : SplitIterator)(itr.str, itr.splitter, itr.limit, itr.keepempty)
end

@vtjnash
Copy link
Member

vtjnash commented Oct 8, 2024

No, even then it is not the same, here's another counter-example:

julia> eachrsplit("abbbcbbbdf", "bb") |> collect
3-element Vector{SubString{String}}:
 "df"
 "cb"
 "ab"

julia> eachsplit("abbbcbbbdf", "bb") |> collect
3-element Vector{SubString{String}}:
 "a"
 "bc"
 "bdf"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Indicates new feature / enhancement requests iteration Involves iteration or the iteration protocol strings "Strings!"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add eachrsplit iterator

4 participants