classification
Title: Add keyword args support to str/bytes.expandtabs()
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ezio.melotti Nosy List: eric.araujo, ezio.melotti, mark.dickinson, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-04-21 01:19 by ezio.melotti, last changed 2013-11-16 17:18 by ezio.melotti. This issue is now closed.

Files
File name Uploaded Description Edit
expandtabs.diff ezio.melotti, 2013-04-21 01:19 review
Messages (8)
msg187481 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-04-21 01:19
The attached patch adds keyword args support to str/bytes.expandtabs():
>>> 'a\tb'.expandtabs(tabsize=8)
'a       b'
>>> b'a\tb'.expandtabs(tabsize=8)
b'a       b'
msg187927 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-04-27 19:51
Where is the “Approve patch” button :)
msg187929 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-04-27 20:02
Note that adding support of keyword arguments significant slowdown argument parsing. Please measure time of the function call with and without the patch.

I object to add support for keyword arguments in a quick built-in functions. Especially if it's the only argument.
msg187969 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-04-28 07:40
Without patch:
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et()'
1000000 loops, best of 3: 0.672 usec per loop
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et(4)'
1000000 loops, best of 3: 0.744 usec per loop
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et(8)'
1000000 loops, best of 3: 0.762 usec per loop

$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et()'
1000000 loops, best of 3: 0.658 usec per loop
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et(4)'
1000000 loops, best of 3: 0.73 usec per loop
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et(8)'
1000000 loops, best of 3: 0.769 usec per loop
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et(tabsize=4)'
1000000 loops, best of 3: 1.84 usec per loop
$ ./python -m timeit -s 'et = "a\tb\tc\td\te".expandtabs' 'et(tabsize=8)'
1000000 loops, best of 3: 1.89 usec per loop

If "tabsize" is not used the performances seem the same, if it's used it's 2-3 times slower.  I don't think expandtabs is used in performance-critical paths, but if it is the patch shouldn't affect it as long as people don't add "tabsize" to the call.

FTR the reason to add this is consistency (Python functions allow you to pass positional args as keywords too) and readability (s.expandtabs(3) might be read as "expand at most 3 tabs" or something else).
msg188353 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-05-04 11:48
> FTR the reason to add this is consistency

This always was a weak reason.

> and readability

It is a matter of taste. For me s.expandtabs(3) looks more readable than s.expandtabs(tabsize=3).

I don't see an urgent need for this feature and don't like a complication. Let's defer this change until the passing of the keyword arguments will not be significantly optimized. I'm -0 for committing right now.
msg188402 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2013-05-04 20:03
> For me s.expandtabs(3) looks more readable than s.expandtabs(tabsize=3).

I disagree:  I think the second is more readable, and I think it's especially helpful to those not intimately familiar with the language (which probably accounts for the vast majority of Python users) to see code like "s.expandtabs(tabsize=3)", or "int(43, base=8)", or "s.splitlines(keepends=True)":  such code is instantly understandable, whereas someone seeing "s.splitlines(True)" likely has to stop and wonder what the "True" is doing.
msg203064 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-11-16 16:59
Ezio, you can commit the patch if you want.
msg203065 - (view) Author: Roundup Robot (python-dev) Date: 2013-11-16 17:11
New changeset 82b58807f481 by Ezio Melotti in branch 'default':
#17806: Added keyword-argument support for "tabsize" to str/bytes.expandtabs().
http://hg.python.org/cpython/rev/82b58807f481
History
Date User Action Args
2013-11-16 17:18:12ezio.melottisetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2013-11-16 17:11:11python-devsetnosy: + python-dev
messages: + msg203065
2013-11-16 16:59:40serhiy.storchakasetmessages: + msg203064
stage: patch review -> commit review
2013-05-04 20:03:05mark.dickinsonsetmessages: + msg188402
2013-05-04 11:48:36serhiy.storchakasetmessages: + msg188353
2013-04-28 07:40:28ezio.melottisetmessages: + msg187969
2013-04-27 20:02:40serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg187929
2013-04-27 19:51:15eric.araujosetmessages: + msg187927
2013-04-27 16:35:07ezio.melottisetnosy: + mark.dickinson, eric.araujo
2013-04-21 01:19:14ezio.melotticreate