python / cpython

The Python programming language
https://www.python.org
Other
63.35k stars 30.34k forks source link

turtledemo/__main__.py: SyntaxError: invalid string prefix else"#fca" #84611

Closed bc75918c-a209-4fa3-b6cf-28cfb7317f76 closed 4 years ago

bc75918c-a209-4fa3-b6cf-28cfb7317f76 commented 4 years ago
BPO 40431
Nosy @terryjreedy, @vstinner, @encukou, @hroncok, @miss-islington, @aeros
PRs
  • python/cpython#19777
  • python/cpython#19784
  • python/cpython#19789
  • 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: ```python assignee = None closed_at = created_at = labels = ['3.8', '3.9'] title = 'turtledemo/__main__.py: SyntaxError: invalid string prefix else"#fca"' updated_at = user = 'https://github.com/hroncok' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'none' closed = True closed_date = closer = 'aeros' components = ['Demos and Tools'] creation = creator = 'hroncok' dependencies = [] files = [] hgrepos = [] issue_num = 40431 keywords = ['patch'] message_count = 12.0 messages = ['367575', '367581', '367583', '367585', '367590', '367607', '367609', '367612', '367618', '367638', '367648', '369154'] nosy_count = 6.0 nosy_names = ['terry.reedy', 'vstinner', 'petr.viktorin', 'hroncok', 'miss-islington', 'aeros'] pr_nums = ['19777', '19784', '19789'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue40431' versions = ['Python 3.8', 'Python 3.9'] ```

    bc75918c-a209-4fa3-b6cf-28cfb7317f76 commented 4 years ago

    With Python 3.9.0a6 we get the following syntax error when bytecompiling the standard library in Fedora:

    Compiling '/usr/lib64/python3.9/turtledemo/main.py'... *** File "/usr/lib64/python3.9/turtledemo/main.py", line 275 bg="#d00" if clear == NORMAL else"#fca") ^ SyntaxError: invalid string prefix

    I've looked and the bad code is there for all branches, but only with 3.9.0a6 I get the SyntaxError.

    I wonder whether this is a know new SyntaxError or not. This "worked" with 3.9.0a5:

    >>> "yes" if False else"no"
    'no'

    Happy to submit a PR for turtledemo.

    vstinner commented 4 years ago

    New changeset 49f70db83e2c62ad06805927f53f6c3e8f4b798e by Miro Hrončok in branch 'master': bpo-40431: Fix syntax typo in turtledemo (GH-19777) https://github.com/python/cpython/commit/49f70db83e2c62ad06805927f53f6c3e8f4b798e

    vstinner commented 4 years ago

    I don't think that it's worth it to backport the change to 3.7 and 3.8: in Python 3.7 and 3.8, the code is accepted.

    I also reported the issue to: https://bugs.python.org/issue40334#msg367580

    Miro: are you ok to close the issue (not backport)?

    bc75918c-a209-4fa3-b6cf-28cfb7317f76 commented 4 years ago

    I am OK. I don't see why not to backport it, but I don't care much.

    vstinner commented 4 years ago

    I am OK. I don't see why not to backport it, but I don't care much.

    Python 3.7 and 3.8 are not broken, so there is no need to fix them :-) Thanks for the fix.

    I reported the issue to: https://bugs.python.org/issue40246#msg367587

    aeros commented 4 years ago

    I don't think that it's worth it to backport the change to 3.7 and 3.8: in Python 3.7 and 3.8, the code is accepted.

    Python 3.7 and 3.8 are not broken, so there is no need to fix them :-)

    I think we could very reasonably change else"#fca" -> else "#fca" on the current bugfix branches, or at least 3.8. Even when it doesn't cause a syntax error and the attempted prefix is ignored, it's still more syntactically correct compared to attempting to use an "e-string" (which of course doesn't exist) and has nearly zero cost on our end to fix.

    While it's a low priority issue since it doesn't raise an error, that doesn't necessarily mean it's correct. In the latest stable version of the stdlib, we should aim to provide a good example IMO, especially if it comes at practically zero cost to us.

    vstinner commented 4 years ago

    If you backport the change, please remove the NEWS entry, since it's not a SyntaxError in 3.7 and 3.8.

    aeros commented 4 years ago

    Victor Stinner wrote:

    If you backport the change, please remove the NEWS entry, since it's not a SyntaxError in 3.7 and 3.8.

    Good point. In that case, I'll do a manual backport of the PR just to 3.8 and skip the news entry. As mentioned above, I think most of the value in fixing it when the SyntaxError isn't present is mainly just for providing a good example in the latest stable version of the stdlib, so I don't think there would be much additional value in also backporting it to 3.7.

    aeros commented 4 years ago

    New changeset cc011b5190b63f0be561ddec38fc4cd9e60cbf6a by Kyle Stanley in branch '3.8': [3.8] bpo-40431: Fix syntax typo in turtledemo (GH-19777) (bpo-19784) https://github.com/python/cpython/commit/cc011b5190b63f0be561ddec38fc4cd9e60cbf6a

    miss-islington commented 4 years ago

    New changeset adb1f853482e75e81ae0ae7307318a1051ca46b5 by Miss Islington (bot) in branch '3.7': [3.8] bpo-40431: Fix syntax typo in turtledemo (GH-19777) (GH-19784) https://github.com/python/cpython/commit/adb1f853482e75e81ae0ae7307318a1051ca46b5

    vstinner commented 4 years ago

    Ok, the typo is now fixed in 3.7, 3.8 and master branches ;-) Thanks Miro for the bug reportand Kyle for the backport.

    terryjreedy commented 4 years ago

    I believe the file was synchronized across versions as new features have not recently been added. Backporting the fix to my typo should keep it that way. This is an example of why code review even of patches that 'work' is a good thing.