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

Show expected input in custom "print" error message #74782

Closed
ncoghlan opened this issue Jun 8, 2017 · 16 comments
Closed

Show expected input in custom "print" error message #74782

ncoghlan opened this issue Jun 8, 2017 · 16 comments
Assignees
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

ncoghlan commented Jun 8, 2017

BPO 30597
Nosy @ncoghlan, @ericvsmith, @ned-deily, @serhiy-storchaka, @CuriousLearner
PRs
  • bpo-30597: Shows expected input in custom 'print' error message #2009
  • [3.6] bpo-30597: Show expected input in custom 'print' error message #2531
  • 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 = 'https://github.com/ncoghlan'
    closed_at = <Date 2017-07-03.07:50:53.650>
    created_at = <Date 2017-06-08.10:36:46.567>
    labels = ['interpreter-core', 'type-feature', '3.7']
    title = 'Show expected input in custom "print" error message'
    updated_at = <Date 2017-07-03.07:50:53.649>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2017-07-03.07:50:53.649>
    actor = 'ncoghlan'
    assignee = 'ncoghlan'
    closed = True
    closed_date = <Date 2017-07-03.07:50:53.650>
    closer = 'ncoghlan'
    components = ['Interpreter Core']
    creation = <Date 2017-06-08.10:36:46.567>
    creator = 'ncoghlan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 30597
    keywords = []
    message_count = 16.0
    messages = ['295397', '295400', '295412', '295439', '295444', '295483', '295484', '295485', '296424', '296429', '296516', '296523', '296524', '297467', '297534', '297535']
    nosy_count = 5.0
    nosy_names = ['ncoghlan', 'eric.smith', 'ned.deily', 'serhiy.storchaka', 'CuriousLearner']
    pr_nums = ['2009', '2531']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue30597'
    versions = ['Python 3.6', 'Python 3.7']

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 8, 2017

    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)

    @ncoghlan ncoghlan added 3.7 (EOL) end of life type-feature A feature request or enhancement labels Jun 8, 2017
    @CuriousLearner
    Copy link
    Member

    Hi Nick,

    Can I work on this issue?

    @ericvsmith
    Copy link
    Member

    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",

    ?

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 8, 2017

    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).

    @ericvsmith
    Copy link
    Member

    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)'?

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 9, 2017

    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.

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 9, 2017

    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=' '".

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 9, 2017

    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'

    @serhiy-storchaka serhiy-storchaka added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jun 20, 2017
    @serhiy-storchaka
    Copy link
    Member

    New changeset 3a7f035 by Serhiy Storchaka (Sanyam Khurana) in branch 'master':
    bpo-30597: Show expected input in custom 'print' error message. (bpo-2009)
    3a7f035

    @serhiy-storchaka
    Copy link
    Member

    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.

    @ncoghlan
    Copy link
    Contributor Author

    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.

    @ncoghlan
    Copy link
    Contributor Author

    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.

    @ncoghlan ncoghlan reopened this Jun 21, 2017
    @ncoghlan
    Copy link
    Contributor Author

    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.

    @ned-deily
    Copy link
    Member

    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!

    @ncoghlan ncoghlan self-assigned this Jul 2, 2017
    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jul 3, 2017

    New changeset bfdc6fd by Nick Coghlan in branch '3.6':
    [3.6] bpo-30597: Show expected input in custom 'print' error message. (GH-2531)
    bfdc6fd

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jul 3, 2017

    And merged. Thanks all!

    @ncoghlan ncoghlan closed this as completed Jul 3, 2017
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants