Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Add eachrsplit iterator #51646

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Conversation

jakobnissen
Copy link
Contributor

@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
Contributor 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
2 checks passed
@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