Move casting prompt after its validation in _raw_input()

[myungsekyo]

BPO	37466
Nosy	@taleinat, @stevendaprano, @MyungSeKyo
PRs	bpo-37466: Move casting prompt after its validation in _raw_input() #14502

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-07-03.01:02:12.831>
created_at = <Date 2019-07-01.09:35:33.279>
labels = ['invalid', 'expert-IO', 'performance']
title = 'Move casting prompt after its validation in _raw_input()'
updated_at = <Date 2019-07-03.01:02:12.827>
user = 'https://github.com/myungsekyo'

bugs.python.org fields:

activity = <Date 2019-07-03.01:02:12.827>
actor = 'myungsekyo'
assignee = 'none'
closed = True
closed_date = <Date 2019-07-03.01:02:12.831>
closer = 'myungsekyo'
components = ['IO']
creation = <Date 2019-07-01.09:35:33.279>
creator = 'myungsekyo'
dependencies = []
files = []
hgrepos = []
issue_num = 37466
keywords = ['patch']
message_count = 5.0
messages = ['346986', '346989', '346990', '347003', '347173']
nosy_count = 3.0
nosy_names = ['taleinat', 'steven.daprano', 'myungsekyo']
pr_nums = ['14502']
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue37466'
versions = []

[myungsekyo]

I think it is more efficient to cast variable prompt into string after its validation in _raw_input().
Because If it is None or empty string then it is not used.

import timeit

MAX = 10000000


start = timeit.default_timer()
for i in range(MAX):
    _raw_input()

end = timeit.default_timer()

print('estimated : {}'.format(end - start))

I tested on 10 millions inputs with above code.
and the result is as follows.

Before:
estimated : 5.060587857999053
estimated : 5.0425375679988065
estimated : 4.850400277999142
estimated : 4.888060468998447
estimated : 4.849542597999971
estimated : 4.822679259999859
estimated : 5.053791769001691
estimated : 4.914149145999545
estimated : 4.9584080040003755
estimated : 4.944711199001176

After:
estimated : 4.014042392998817
estimated : 3.987057284997718
estimated : 4.081281360999128
estimated : 4.06813505899845
estimated : 4.040622504999192
estimated : 4.1239150339970365
estimated : 4.174400065003283
estimated : 4.015272281998477
estimated : 4.034917910001241
estimated : 4.08582956799728

[taleinat]

This is a subtle change with the potential to affect more than just performance. Also, in this case, the performance is likely negligible in every actual use-case.

This might actually be more correct, though. For example, one would expect passing prompt=None to result in no prompt displayed, rather than "None".

[stevendaprano]

I don't think that will work. If the user passes a non-string which is falsey, your patch will attempt to write it directly to the stream without converting it to a string.

Try _raw_input(0) as an example.

Tal Einat:

one would expect passing prompt=None to result in no prompt displayed

I wouldn't. That's not how getpass works now, or input.

By the way, myungsekyo, that's not the best way to use timeit to time code snippets. You are timing the overhead of the loop, ten million lookups of the name "_raw_input", and most importantly, ten million times that _raw_input halts waiting for input from the user. Surely you didn't sit there hitting Enter over and over again?

[taleinat]

> one would expect passing prompt=None to result in no prompt displayed

I wouldn't. That's not how getpass works now, or input.

Ah, thanks for the input, Steve. In that case, this should likely not be changed.

[myungsekyo]

Thanks to your reviews!
I understood what you mean.
This patch is unnecessary.
I will close this issue.