Issue28937
Created on 2016-12-11 15:11 by barry, last changed 2021-01-12 14:05 by corona10.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
split_prune_1.patch | abarry, 2016-12-12 16:42 | review |
Messages (31) | |||
---|---|---|---|
msg282923 - (view) | Author: Barry A. Warsaw (barry) * ![]() |
Date: 2016-12-11 15:11 | |
This has finally bugged me enough to file an issue, although I wouldn't be able to use it until Python 3.7. There's a subtle but documented difference in str.split() when sep=None: >>> help(''.split) Help on built-in function split: split(...) method of builtins.str instance S.split(sep=None, maxsplit=-1) -> list of strings Return a list of the words in S, using sep as the delimiter string. If maxsplit is given, at most maxsplit splits are done. If sep is not specified or is None, any whitespace string is a separator and empty strings are removed from the result. I.e., that empty strings are removed from the result. This does not happen when sep is given, leading to this type of unfortunate code: >>> 'foo,bar,baz'.split(',') ['foo', 'bar', 'baz'] >>> 'foo,bar,baz'.replace(',', ' ').split() ['foo', 'bar', 'baz'] >>> ''.split(',') [''] >>> ''.replace(',', ' ').split() [] Specifically, code that wants to split on say commas, but has to handle the case where the source string is empty, shouldn't have to also filter out the single empty string item. Obviously we can't change existing behavior, so I propose to add a keyword argument `prune` that would make these two bits of code identical: >>> ''.split() [] >>> ''.split(' ', prune=True) [] and would handle the case of ''.split(',') without having to resort to creating an ephemeral intermediate string. `prune` should be a keyword-only argument, defaulting to False. |
|||
msg282925 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-11 15:26 | |
I understand the feeling. However, in a project I maintain, we want the other way around - to be able to never have an empty list, even if the string is empty (we resorted to using re.split in the end, which has this behaviour). Consider: rest = re.split(" +", rest)[0].strip() This gives us None-like behaviour in splitting, at the cost of not actually using str.split. I'm +1 on the idea, but I'd like some way to better generalize str.split use (not everyone knows you can pass None and/or an integer). (At the same time, the counter arguments where str has too many methods, or that methods shouldn't do too much, also apply here.) But I don't like bikeshedding too much, so let's just count me as +1 for your way, if there's no strong momentum for mine :) |
|||
msg282926 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2016-12-11 15:32 | |
Current behavior is consistent with str.count(): len(string.split(sep)) == string.count(sep) + 1 and re.split(): re.split(re.escape(sep), string) == string.split(sep) May be the behavior when sep is None should be changed for consistency with the behavior when sep is not None? |
|||
msg282927 - (view) | Author: Barry A. Warsaw (barry) * ![]() |
Date: 2016-12-11 15:35 | |
On Dec 11, 2016, at 03:32 PM, Serhiy Storchaka wrote: >Current behavior is consistent with str.count(): > > len(string.split(sep)) == string.count(sep) + 1 > >and re.split(): > > re.split(re.escape(sep), string) == string.split(sep) Yep. My suggestion is a straight up 'practicality beats purity' request. >May be the behavior when sep is None should be changed for consistency with >the behavior when sep is not None? I'm very strongly -1 on changing any existing behavior. |
|||
msg282928 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-11 15:38 | |
Changing the behaviour when sep is None is a big backwards-compatibility break, and I'm not sure we'd even want that. It's logical to allow passing None to mean the same thing as NULL (i.e. no arguments), and the behaviour in that case has been like that for... well, long enough that changing it isn't really feasible. I agree with Barry here, especially since this is a completely opt-in feature, and existing behaviour isn't changed without the user's knowledge. |
|||
msg282929 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2016-12-11 15:57 | |
I meant adding boolean argument that changes the behavior when sep is None, not when it is not None. |
|||
msg282930 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-11 16:01 | |
That would work for my case, but it wouldn't for Barry's (unless I missed something). He wants a non-None argument to not leave empty strings, but I want a None argument to leave empty strings... I don't think there's a one-size-fits-all solution in this case, but feel free to prove me wrong :) |
|||
msg282931 - (view) | Author: Barry A. Warsaw (barry) * ![]() |
Date: 2016-12-11 16:03 | |
On Dec 11, 2016, at 03:57 PM, Serhiy Storchaka wrote: >I meant adding boolean argument that changes the behavior when sep is None, >not when it is not None. Ah, I understand now, thanks. However, I'm not sure that addresses my particular use case. It's actually kind of handy to filter out the empty strings. But I'm open to counter arguments. |
|||
msg282932 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-11 16:29 | |
Actually, there might be a way. We could make prune default to True if sep is None, and default to False if sep is not None. That way, we get to keep the existing behaviour for either case, while satisfying both of our use cases :) If that's a bad idea (and it quite probably is), I'll retract it. But it's an interesting possibility to at least consider. |
|||
msg282935 - (view) | Author: Matthew Barnett (mrabarnett) * ![]() |
Date: 2016-12-11 18:17 | |
So prune would default to None? None means current behaviour (prune if sep is None else don't prune) True means prune empty strings False means don't prune empty string |
|||
msg282936 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2016-12-11 19:00 | |
A few randomly ordered thoughts about splitting: * The best general purpose text splitter I've ever seen is in MS Excel and is called "Text to Columns". It has a boolean flag, "treat consecutive delimiters as one" which is off by default. * There is a nice discussion on the complexities of the current design on StackOverflow: http://stackoverflow.com/questions/16645083 In addition, there are many other SO questions about the behavior of str.split(). * The learning curve for str.split() is already high. The doc entry for it has been revised many times to try and explain what it does. I'm concerned that adding another algorithmic option to it may make it more difficult to learn and use in the common cases (API design principle: giving users more options can impair usability). Usually in Python courses, I recommend using str.split() for the simple, common cases and using regex when you need more control. * What I do like about the proposal is that that there is no clean way to take the default whitespace splitting algorithm and customize to a particular subset of whitespace (i.e. tabs only). * A tangential issue is that it was a mistake to expose the maxsplit=-1 implementation detail. In Python 2.7, the help was "S.split([sep [,maxsplit]])". But folks implementing the argument clinic have no way of coping with optional arguments that don't have a default value (like dict.pop), so they changed the API so that the implementation detail was exposed, "S.split(sep=None, maxsplit=-1)". IMO, this is an API regression. We really don't want people passing in -1 to indicate that there are no limits. The Python way would have been to use None as a default or to stick with the existing API where the number of arguments supplied is part of the API (much like type() has two different meanings depending on whether it has an arity of 1 or 3). Overall, I'm +0 on the proposal but there should be good consideration given to 1) whether there is a sufficient need to warrant increasing API complexity, making split() more difficult to learn and remember, 2) considering whether "prune" is the right word (can someone who didn't write the code read it clearly afterwards), 3) or addressing this through documentation (i.e. showing the simple regexes needed for cases not covered by str.split). |
|||
msg282954 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-11 23:58 | |
Matthew: Yes, that's exactly the way I was going about it. Thank you Raymond for your comments (and your informative answer on that SO question). I think that part of the problem is that no delimiter (or None) behaves differently than with a delimiter. If we wanted proper consistency, we would have needed to make passing None (or nothing) the same as passing whitespace, but alas, we have to work with what we have. As you said, API complexity is a concern that needs to be addressed. I think that the most important part is how it's documented, and, if phrased correctly (which is non-trivial), could actually simplify the explanation. Consider this draft: *** The value of the `prune` keyword argument determines whether or not consecutive delimiters should be grouped together. If `prune` is not given or None, it defaults to True if sep is None or not given, and False otherwise. If `prune` is True, consecutive delimiters (all whitespace if None or not given) are regarded as a single separator, and the result will not contain any empty string. The resulting list may be empty. Otherwise, if `prune` is False, consecutive delimiters are not grouped together, and the result may contain one or more empty string. The resulting list will never be empty. *** I may be oversimplifying this, but it seems to me that this might help some people by hopefully streamlining the explanation. This still doesn't solve the issue where a user can't say "split on a space or a tab, but not newlines", which IMO is lacking in the design, but that may be for another issue ;) I've wrapped up a basic patch which probably doesn't work at all; I'll put it up when it's at least partly working for everyone to look at. |
|||
msg282958 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-12 02:31 | |
Here's an initial patch. It works exactly as discussed earlier, doesn't break any tests, and retains full backwards compatibility. No doc changes (except for the docstrings of str.[r]split) and no tests, as this is just a preliminary patch to see if there's any merit to the idea. |
|||
msg282961 - (view) | Author: Sye van der Veen (syeberman) * | Date: 2016-12-12 04:22 | |
In the sep!=None case, there are existing alternatives to prune=True that aren't many more keystrokes: >>> ''.split(' ', prune=True) [] >>> [x for x in ''.split(' ') if x] [] >>> list(filter(bool, ''.split(' '))) # or drop list() and use the iterator directly [] This becomes even fewer keystrokes for users that create a prune() or split_prune() function. For the sep==None case, I agree there are no alternatives to prune=False (aside from rolling your own split function). However, instead of prune, what if sep accepted a tuple of strings, similar to startswith. In this case, each string would be considered one possible, yet distinct, delimiter: >> ''.split(prune=False) [''] >> ''.split((' ', '\t')) # typical whitespace [''] >> ''.split(tuple(string.whitespace)) # ASCII whitespace [''] Once again, this becomes even easier for users that create a split_no_prune() function, or that assign tuple(string.whitespace) to a variable. It would also nicely handle strings with non-homogeneous delimiters: >>> '1?2,,3;'.split((',', ';', '?')) ['1', '2', '', '3', ''] I personally find the 0-argument str.split() one of the great joys of Python. It's common to have to split out words from a sentence, and having that functionality just 8 characters away at all times has been very useful. |
|||
msg282962 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-12 04:38 | |
Yes, I agree that being able to pass in a tuple would be really useful. As far as rolling out a custom function goes, I'd sooner reach for re.split than do that, so I don't really have a strong argument for either side. Feel free to play with the patch or make an entirely new one, though! I mainly submitted the patch to keep the discussion going, and eventually come to a concensus, but I don't have a strong opinion either way :) |
|||
msg282963 - (view) | Author: Vedran Čačić (veky) * | Date: 2016-12-12 05:09 | |
The problem with .split is its split (pun intended) personality: it really is two functions that have separate use cases, and have different algorithms; and people think about them as separate functions. In that view, we have just fallen afoul of Guido's rule of no literal passing bool arguments. The true solution would probably be to bite the bullet and have two separate methods. After all, .splitlines is a separate method for precisely such a reason. |
|||
msg282966 - (view) | Author: Raymond Hettinger (rhettinger) * ![]() |
Date: 2016-12-12 06:18 | |
Guido, do you have an option on this? IIRC, this was an API you created. Nick's thought (posted on twitter) is that 'filter(None, sep.split(input)' already covers the "drop the empty values" case. My feelings are mixed. Though I've never needed in practice, it would be nice if the whitespace removal algorithm could be customized to just a space or just a tab. On the other hand, I think the new parameter would make the API more confusing and harder to learn. It might be better to just document either the filter(None) approach or a simple regex for the less common cases. |
|||
msg283011 - (view) | Author: Barry A. Warsaw (barry) * ![]() |
Date: 2016-12-12 15:32 | |
I really appreciate all the feedback. Here are some thoughts. I'm well aware of the filter(), re, and other options, and certainly those can be made to work, but they're non-obvious. The reason I suggested an enhancement to str.split() is because I've seen the replace().split() being used far too often, and what I think is happening is that people take the most natural path to accomplish their goals: they know they just want to do a simple string split on a token (usually one character) so they start out with the obvious str.split(',') or whatever. Then they notice that it doesn't work consistent with their mental model in some corner cases. The next common step isn't from there to filter() or re. The former isn't a well-known API and the latter is viewed as "too complex". Their next mental step is "oh, so providing a sep has different behavior that I don't want, so I'll just replace the comma with a space and now don't have to provide sep". And now str.split() does what they want. Done. Move along. I do wish the str.split() API was consistent w.r.t. to sep=None, but it's what we have and is a very well known API. @rhettinger: I'm of mixed opinion on it too! I really wanted to get this in the tracker and see if we could come up with something better, but so far I still like `prune` the best. @ebarry: Thanks for the draft docs, but that's not how I think about this. I'd be utilitarian and get right to the point, e.g.: """ The value of `prune` controls whether empty strings are removed from the resulting list. The default value (None) says to use the default behavior, which for backward compatibility reasons is different whether sep is None or not (see above). Regardless of the value of sep, when prune is True empty strings are removed and when prune is False they are not. """ So @mrabarnett, +1 on the suggested defaults. Lastly, as for Guido's admonition against boolean arguments, I would make prune a keyword-only argument, so that forces the code to be readable and should alleviate those concerns. The trade-off is the extra typing, but that's actually besides the point. The win here is that the solution is easily discoverable and avoids the intermediate string object. |
|||
msg283019 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2016-12-12 16:16 | |
I like the proposal. I agree that filter(None, ...) is not discoverable (and has its own magic). So the proposal would be: prune=False -> empty strings stay, prune=True, empty strings are dropped, prune=None (default) use True if sep is None, False otherwise. Right? Some end cases: - ''.split(None, prune=True) -> [''] - 'x x'.split(None, prune=True) -> ['x', '', 'x'] Right? While we're here I wish there was a specific argument we could translate .split(None) into, e.g. x.split() == x.split((' ', '\t', '\n', '\r', '\f')) # or whatever set of strings |
|||
msg283021 - (view) | Author: Barry A. Warsaw (barry) * ![]() |
Date: 2016-12-12 16:26 | |
On Dec 12, 2016, at 04:16 PM, Guido van Rossum wrote: >So the proposal would be: prune=False -> empty strings stay, prune=True, >empty strings are dropped, prune=None (default) use True if sep is None, >False otherwise. Right? Yep! >Some end cases: > >- ''.split(None, prune=True) -> [''] >- 'x x'.split(None, prune=True) -> ['x', '', 'x'] > >Right? Isn't that what you'd expect if prune=False instead? (i.e. prune=True always drops empty strings from the results) >While we're here I wish there was a specific argument we could translate >.split(None) into, e.g. x.split() == x.split((' ', '\t', '\n', '\r', '\f')) # >or whatever set of strings Is that the sep=<some tuple> idea that @syeberman suggested earlier? If so, then you could do: >>> x.split(tuple(string.whitespace)) Would that suffice? |
|||
msg283025 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2016-12-12 16:42 | |
Barry: Sure, the docs example was just a quick write-up, you can word it however you want! Guido: Pretty much, except the other way around (when prune is False, i.e. "don't remove empty strings"). The attached patch exposes the behaviour (it's identical to last night's, but I'm re-uploading it as an unrelated file went in), except that the `prune` argument isn't keyword-only (I didn't know how to do this, and didn't bother searching for just a proof-of-concept). |
|||
msg283029 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2016-12-12 17:12 | |
> except the other way around Whoops. Indeed. So all's well here. > x.split(tuple(string.whitespace)) Yes, that's what I was after. (But it can be a separate PR.) |
|||
msg283031 - (view) | Author: Vedran Čačić (veky) * | Date: 2016-12-12 17:28 | |
I think Guido's mistake is relevant here. It tripped me too. Too much negatives, and "prune" is not really well-known verb. Besides, we already have str.splitlines' keepends, which works the opposite way. |
|||
msg338412 - (view) | Author: Cheryl Sabella (cheryl.sabella) * ![]() |
Date: 2019-03-19 22:34 | |
@ebarry, any interest in converting your patch to a GitHub pull request? Thanks! |
|||
msg338422 - (view) | Author: Barry A. Warsaw (barry) * ![]() |
Date: 2019-03-19 23:41 | |
@veky - Thank you for pointing out splitlines(keepends=True). If we wanted consistency, then we'd change the sense and use something like .split(keepempty=True), however: * I don't like run-on names, so I would suggest keep_empty * Maybe just `keep` is enough * Either way, this should be a keyword only argument * The default would still be None (i.e. current behavior), but keep_empty=True would be equivalent to prune=False and keep_empty=False would be equivalent to prune=True in the previous discussion. |
|||
msg338429 - (view) | Author: Anilyka Barry (abarry) * ![]() |
Date: 2019-03-20 01:41 | |
Unfortunately not. I no longer have the time or means to work on this, sorry. I hope someone else can pick it up. |
|||
msg354907 - (view) | Author: Philippe Cloutier (Philippe Cloutier) | Date: 2019-10-18 16:10 | |
I understood the current (only) behavior, but coming from a PHP background, I really didn't expect it. Thank you for this request, I would definitely like the ability to get behavior matching PHP's explode(). |
|||
msg354908 - (view) | Author: Philippe Cloutier (Philippe Cloutier) | Date: 2019-10-18 16:26 | |
I assume the "workaround" suggested by Raymond in msg282966 is supposed to read... filter(None, str.split(sep) ... rather than filter(None, sep.split(input)). |
|||
msg384306 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2021-01-04 02:21 | |
This issue probably needs a new champion. There is broad agreement but some bike shedding, so a PEP isn’t needed.-- --Guido (mobile) |
|||
msg384321 - (view) | Author: Zackery Spytz (ZackerySpytz) * ![]() |
Date: 2021-01-04 11:15 | |
I am working on this issue. |
|||
msg384337 - (view) | Author: Guido van Rossum (gvanrossum) * ![]() |
Date: 2021-01-04 17:50 | |
Excellent! |
History | |||
---|---|---|---|
Date | User | Action | Args |
2021-01-12 14:05:00 | corona10 | set | nosy:
+ corona10 |
2021-01-04 17:50:49 | gvanrossum | set | messages: + msg384337 |
2021-01-04 11:15:53 | ZackerySpytz | set | versions:
+ Python 3.10, - Python 3.8 nosy: + ZackerySpytz messages: + msg384321 assignee: ZackerySpytz |
2021-01-04 03:37:27 | rhettinger | set | nosy:
- rhettinger |
2021-01-04 02:21:42 | gvanrossum | set | messages: + msg384306 |
2021-01-04 02:06:27 | karlcow | set | nosy:
+ karlcow |
2019-10-18 16:26:59 | Philippe Cloutier | set | messages: + msg354908 |
2019-10-18 16:10:05 | Philippe Cloutier | set | nosy:
+ Philippe Cloutier messages: + msg354907 title: str.split(): remove empty strings when sep is not None -> str.split(): allow removing empty strings (when sep is not None) |
2019-03-20 01:41:46 | abarry | set | nosy:
- abarry |
2019-03-20 01:41:30 | abarry | set | messages: + msg338429 |
2019-03-19 23:41:08 | barry | set | messages: + msg338422 |
2019-03-19 22:34:53 | cheryl.sabella | set | nosy:
+ cheryl.sabella messages: + msg338412 versions: + Python 3.8, - Python 3.7 |
2016-12-12 17:28:41 | veky | set | messages: + msg283031 |
2016-12-12 17:12:14 | gvanrossum | set | messages: + msg283029 |
2016-12-12 16:42:20 | abarry | set | files:
+ split_prune_1.patch messages: + msg283025 |
2016-12-12 16:41:55 | abarry | set | files: - split_prune_1.patch |
2016-12-12 16:26:08 | barry | set | messages: + msg283021 |
2016-12-12 16:16:06 | gvanrossum | set | messages: + msg283019 |
2016-12-12 15:32:35 | barry | set | messages: + msg283011 |
2016-12-12 06:18:20 | rhettinger | set | nosy:
+ gvanrossum messages: + msg282966 |
2016-12-12 05:09:27 | veky | set | nosy:
+ veky messages: + msg282963 |
2016-12-12 04:38:29 | abarry | set | messages: + msg282962 |
2016-12-12 04:22:01 | syeberman | set | nosy:
+ syeberman messages: + msg282961 |
2016-12-12 02:31:57 | abarry | set | files:
+ split_prune_1.patch keywords: + patch messages: + msg282958 stage: test needed |
2016-12-11 23:58:37 | abarry | set | messages: + msg282954 |
2016-12-11 19:00:22 | rhettinger | set | nosy:
+ rhettinger messages: + msg282936 |
2016-12-11 18:17:02 | mrabarnett | set | nosy:
+ mrabarnett messages: + msg282935 |
2016-12-11 16:29:47 | abarry | set | messages: + msg282932 |
2016-12-11 16:03:13 | barry | set | messages: + msg282931 |
2016-12-11 16:01:56 | abarry | set | messages: + msg282930 |
2016-12-11 15:57:39 | serhiy.storchaka | set | messages: + msg282929 |
2016-12-11 15:38:20 | abarry | set | messages: + msg282928 |
2016-12-11 15:35:54 | barry | set | messages: + msg282927 |
2016-12-11 15:32:40 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg282926 |
2016-12-11 15:26:42 | abarry | set | type: enhancement messages: + msg282925 nosy: + abarry |
2016-12-11 15:11:42 | barry | create |