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

Rsplit #385

Closed
wants to merge 3 commits into from
Closed

Rsplit #385

wants to merge 3 commits into from

Conversation

jferard
Copy link
Contributor

@jferard jferard commented Feb 8, 2020

See #381
There where different options: a) use rlocate, b) try to insert the rsplit logic directly in the code or c) merge the first chunks to keep only the last chunks. Options a) and b) had a lot of edges cases, so I decided to:

  1. create all the chunks;
  2. merge all chunks excepted maxsplit.

I added two helper functions.

(I used the parameter name rsplit, but it is easy to change this).

split_when functions.
Add two helper functions to aggregate the first elements of
an iterable.
@bbayles
Copy link
Collaborator

bbayles commented Feb 11, 2020

Thanks! It will be a few days before I can review this thoroughly.

@bbayles
Copy link
Collaborator

bbayles commented Feb 17, 2020

This is good work, so thanks for the PR.

The reversal logic isn't quite clear to me. I'll spend some more time with it, but I'm wondering if it might be better to just do the keep_separators option and introduce a helper function in a different PR.

@@ -1112,6 +1113,55 @@ def test_seperators(self):
self.comp_with_str_split(s, delim, maxsplit)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Back on the earlier keep_separator PR, I should have asked this: your function does this:

>>> from more_itertools import split_at
... 
... list(split_at('abccba', lambda x: x == 'a', keep_separator=True))
[[], ['a'], ['b', 'c', 'c', 'b'], ['a'], []]

Is that really what we want, the blanks at the ends?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late answer. The closest example I found is:

>>> import re
>>> re.split("(a)", 'abccba')
['', 'a', 'bccb', 'a', '']

I think it's a strong argument in favour of the current behavior.

@bbayles
Copy link
Collaborator

bbayles commented Mar 6, 2020

I'm going back and forth about the reversal stuff - my problem, not yours! I'm going to pull your keep_separator part out and try to think about the rsplit parts independently.

@bbayles
Copy link
Collaborator

bbayles commented Dec 29, 2020

I'm going to close this one due to age. The reversal stuff is a nice addition, but the complexity it adds to the implementation is more than I want to take on. Thanks!

@bbayles bbayles closed this Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants