msg295397 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-08 10:36 |
Back when we added the custom error message for missing parentheses in print calls, I also pre-seeded an explanation of the error on Stack Overflow: https://stackoverflow.com/questions/25445439/what-does-syntaxerror-missing-parentheses-in-call-to-print-mean-in-python/
While that does seem to be having the desired effect of explaining an otherwise cryptic message to people, William Brown pointed out that it could potentially be improved by taking advantage of the fact we have access to the rest of the line when working out the exact error text:
```
>>> print "Hello world!"
File "<stdin>", line 1
print "Hello world!"
^
SyntaxError: Missing parentheses in call to 'print'. Did you mean 'print("Hello world!")'?
```
The rationale for such a change is similar to the rationale for adding the custom error message in the first place: the folks most likely to hit this are either attempting to run a Python 2 script on Py3, or else attempting to follow a Py2 tutorial on Py3, and *telling* them what's wrong isn't as clear as *showing* them (by reprinting the line with the parentheses added)
|
msg295400 - (view) |
Author: Sanyam Khurana (CuriousLearner) * |
Date: 2017-06-08 10:48 |
Hi Nick,
Can I work on this issue?
|
msg295412 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2017-06-08 11:37 |
You'll need to be careful about fully translating to python 3, though. What do you do with:
print >> sys.stderr, "message"
or:
print "message",
?
|
msg295439 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-08 12:57 |
Those are easy enough to check with strncmp() even in C, so we could make them manageable by just including "<expr>" in the error message, rather than the full text of the original statement.
That is:
Did you mean 'print(<expr>)'? # Default
Did you mean 'print(<expr>, file=<stream>)'? # starts with '>>'
Did you mean 'print(<expr>, end=' ')'? # ends with ','
That's basically correct even when printing multiple arguments, so I'd avoid the temptation to detect commas within the line (particularly since they might be inside a string).
|
msg295444 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2017-06-08 14:58 |
That sounds good to me.
Just to elaborate on my earlier message, since it might have been too terse:
I think it would be misleading to give in error at:
print >>sys.stderr, "message"
and say:
Did you mean 'print("message")'?
since they're not equivalent. If you can produce the equivalent text in the error message, then that's fine with me:
Did you mean 'print("message", file=sys.stderr)'?
|
msg295483 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-09 04:19 |
Eric, right, I left a couple of logical leaps out of my reply as well. My first thought was the same as yours but then I went:
- that means we need to split on commas
- which means we need to track whether or not we're inside a quoted string or not
- which is probably more complexity than is warranted just to produce a slightly better hint to the user
Result: let's just use "<expr>" as a placeholder for "the thing you were trying to print"
Although now that I put it that way, "<output>" is probably a better user-oriented placeholder, since "expr" (short for expression) is a very language-developer-oriented term, while "print(<output>)" is closer to pseudocode.
|
msg295484 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-09 04:33 |
Ah, re-reading the comments when reviewing Sanyam's patch, I just remembered the problem with the "print >> sys.stderr, output" case, which is that it isn't a syntax error, it's a runtime type error:
```
>>> print >> sys.stderr, "message"
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: unsupported operand type(s) for >>: 'builtin_function_or_method' and '_io.TextIOWrapper'
```
So if we ever wanted to provide a specialised prompt for that, it would need to go into the right-shift operator implementation as a final check before raising the type error, rather than here.
That reduces the cases we need to handle here to just the default one and the softspace one, where a trailing comma in the statement text gets replaced with ", end=' '".
|
msg295485 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-09 04:36 |
It looks like the current right-shift error message is already distinctive enough to get people to answers explaining the culprit: https://www.google.com.au/search?q=TypeError:+unsupported+operand+type(s)+for+>>:+'builtin_function_or_method'+and+'_io.TextIOWrapper'
|
msg296424 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-06-20 13:31 |
New changeset 3a7f03584ab75afbf5507970711c87042e423bb4 by Serhiy Storchaka (Sanyam Khurana) in branch 'master':
bpo-30597: Show expected input in custom 'print' error message. (#2009)
https://github.com/python/cpython/commit/3a7f03584ab75afbf5507970711c87042e423bb4
|
msg296429 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2017-06-20 13:38 |
Actually end=' ' in Python 3 is not exact replacement of the trailing comma in Python 2. But it covers most of cases. The difference is too subtle and too subtle for the user that makes an error of using Python 2 syntax in Python 3.
|
msg296516 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-21 04:51 |
A note regarding the applicable versions: for the original custom warning, we decided it made sense to change it in a maintenance release, since the generic syntax error really was incredibly cryptic, and we wanted to get the update into the hands of users as quickly as possible.
For this change, it makes more sense to treat it as a normal enhancement, since potentially saving folks a trip to Stack Overflow is a nice improvement, but not a "we should change the reported exception details in a maintenance release" level change.
|
msg296523 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-21 06:28 |
Oops, having written the above comment, I belatedly remembered why I originally suggested this be handled as a maintenance issue rather than as a regular enhancement: I'd like to backport it to the Python 3.6 stack in Fedora, and I think it's better to do that kind of general 2->3 migration UX enhancement upstream (so everyone benefits), rather than as a downstream Fedora-specific patch (which both reaches a smaller audience and is more of a hassle to implement and maintain).
If anyone strongly objects to backporting it, I'm willing to go down the downstream patch path, but otherwise I'll backport it so it also shows up for everyone in 3.6.3 later in the year.
|
msg296524 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-06-21 06:41 |
Also adding Ned to the nosy list, since it's ultimately his call as the 3.6 release manager.
Ned: the context here is that we just landed an improvement to the custom error message for Py2-style print syntax that also says how to fix the problem, rather than just reporting it.
That's technically a new feature, but I'd like to backport it to 3.6 anyway (akin to the 3.4 backport of the original custom error message), as I believe it will make a meaningful usability difference for folks inadvertently running Py2 only code on Python 3.
My specific interest here is the 3.6 stack in Fedora, so we *could* handle this as a downstream patch, but it seems harmless enough that it makes more sense to make the change for everyone, rather than just us.
|
msg297467 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2017-07-01 02:08 |
While it's debatable, I think one can make something of a case for this being a usability bug rather than a new feature. Given the impact to new users of Python 3 and the apparent relative low risk of the change, I'll go out on a bit of a limb and allow it for 3.6.x. Thanks for bringing it up!
|
msg297534 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-07-03 07:49 |
New changeset bfdc6fdc0e76204caf2261f9cfa9d232fc23906a by Nick Coghlan in branch '3.6':
[3.6] bpo-30597: Show expected input in custom 'print' error message. (GH-2531)
https://github.com/python/cpython/commit/bfdc6fdc0e76204caf2261f9cfa9d232fc23906a
|
msg297535 - (view) |
Author: Nick Coghlan (ncoghlan) * |
Date: 2017-07-03 07:50 |
And merged. Thanks all!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:47 | admin | set | github: 74782 |
2017-07-03 07:50:53 | ncoghlan | set | status: open -> closed
messages:
+ msg297535 stage: backport needed -> resolved |
2017-07-03 07:49:52 | ncoghlan | set | messages:
+ msg297534 |
2017-07-02 02:17:54 | ncoghlan | set | pull_requests:
+ pull_request2598 |
2017-07-02 02:01:38 | ncoghlan | set | assignee: ncoghlan |
2017-07-01 02:08:20 | ned.deily | set | messages:
+ msg297467 |
2017-06-21 06:41:23 | ncoghlan | set | nosy:
+ ned.deily messages:
+ msg296524
|
2017-06-21 06:28:27 | ncoghlan | set | status: closed -> open
stage: resolved -> backport needed messages:
+ msg296523 versions:
+ Python 3.6 |
2017-06-21 04:51:41 | ncoghlan | set | messages:
+ msg296516 |
2017-06-20 13:38:36 | serhiy.storchaka | set | messages:
+ msg296429 |
2017-06-20 13:34:12 | serhiy.storchaka | set | status: open -> closed resolution: fixed stage: commit review -> resolved |
2017-06-20 13:31:35 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg296424
|
2017-06-20 13:29:50 | serhiy.storchaka | set | stage: needs patch -> commit review components:
+ Interpreter Core versions:
- Python 3.6 |
2017-06-09 04:36:20 | ncoghlan | set | messages:
+ msg295485 |
2017-06-09 04:33:10 | ncoghlan | set | messages:
+ msg295484 |
2017-06-09 04:19:28 | ncoghlan | set | messages:
+ msg295483 |
2017-06-08 17:53:05 | CuriousLearner | set | pull_requests:
+ pull_request2075 |
2017-06-08 14:58:25 | eric.smith | set | messages:
+ msg295444 |
2017-06-08 12:57:53 | ncoghlan | set | messages:
+ msg295439 |
2017-06-08 11:37:09 | eric.smith | set | nosy:
+ eric.smith messages:
+ msg295412
|
2017-06-08 10:48:56 | CuriousLearner | set | nosy:
+ CuriousLearner messages:
+ msg295400
|
2017-06-08 10:36:46 | ncoghlan | create | |