Skip to content

Improve the warning message for invalid escape sequences #72315

Description

@yan12125
mannequin
BPO 28128
Nosy @rhettinger, @ncoghlan, @ericvsmith, @ned-deily, @ezio-melotti, @bitdancer, @encukou, @vadmium, @serhiy-storchaka, @1st1, @timgraham, @yan12125, @Vgr255
PRs
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • 28128.diff
  • 28128-2.diff
  • 28128-3.diff
  • 28128-3.diff: Regenerated for review
  • 28128-4.diff
  • 28128-5.diff
  • 28128-6.diff
  • 28128-7.diff
  • 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/ericvsmith'
    closed_at = <Date 2016-10-31.19:26:57.914>
    created_at = <Date 2016-09-13.15:10:24.165>
    labels = ['interpreter-core', 'type-feature', '3.7', 'release-blocker']
    title = 'Improve the warning message for invalid escape sequences'
    updated_at = <Date 2017-03-31.16:36:18.468>
    user = 'https://github.com/yan12125'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:18.468>
    actor = 'dstufft'
    assignee = 'eric.smith'
    closed = True
    closed_date = <Date 2016-10-31.19:26:57.914>
    closer = 'eric.smith'
    components = ['Interpreter Core']
    creation = <Date 2016-09-13.15:10:24.165>
    creator = 'yan12125'
    dependencies = []
    files = ['44694', '45284', '45285', '45287', '45288', '45290', '45292', '45293']
    hgrepos = []
    issue_num = 28128
    keywords = ['patch']
    message_count = 55.0
    messages = ['276285', '276286', '276293', '276326', '276341', '276364', '276548', '276560', '276641', '276646', '276658', '276661', '276662', '276663', '276665', '276666', '276668', '276698', '276715', '276716', '276717', '276720', '276728', '276730', '276731', '276785', '277916', '277967', '278251', '279650', '279654', '279724', '279754', '279757', '279759', '279771', '279776', '279778', '279781', '279782', '279783', '279784', '279785', '279789', '279790', '279791', '279793', '279794', '279795', '279796', '279797', '279798', '279799', '279820', '279821']
    nosy_count = 14.0
    nosy_names = ['rhettinger', 'ncoghlan', 'eric.smith', 'ned.deily', 'ezio.melotti', 'r.david.murray', 'petr.viktorin', 'python-dev', 'martin.panter', 'serhiy.storchaka', 'yselivanov', 'Tim.Graham', 'yan12125', 'abarry']
    pr_nums = ['552']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue28128'
    versions = ['Python 3.6', 'Python 3.7']

    Activity

    1. yan12125 commented on Sep 13, 2016

      yan12125mannequin
      MannequinAuthor

      In bpo-27364, invalid escape sequences in string literals are deprecated. Currently the deprecation message is not so useful when fixing lots of files in one or more large projects. For example, I have two files foo.py and bar.py:

      # foo.py
      import bar
      
      # bar.py
      print('\d')
      It gives:
      $ python3.6 -W error foo.py
      Traceback (most recent call last):
        File "foo.py", line 1, in <module>
          import bar
      DeprecationWarning: invalid escape sequence '\d'

      My idea is that the warning message can be improved to provide more information. In http://bugs.python.org/issue27364#msg269373 it's proposed to let a linter check such misuses. It's useful within a single project. For a project that depends on lots of external projects, a linter is not enough. Things are worse when __import__, imp or importlib are involved, or sys.path is modified. I have to either add some codes or use a debugger to show which module is imported.

      For above reasons, I propose to add at least the filename and the line number to the warning message. For example:

      $ ./python -W error foo.py
      Traceback (most recent call last):
        File "foo.py", line 1, in <module>
          import bar
        File "/home/yen/Projects/cpython/build/bar.py", line 1
          print('\d')
               ^
      SyntaxError: (deprecated usage) invalid escape sequence '\d'

      With that I can know which file or project I should blame.

      Added some of reviewers from bpo-27364 to nosy list.

    2. added
      stdlibStandard Library Python modules in the Lib/ directory
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      type-featureA feature request or enhancement
      on Sep 13, 2016
    3. yan12125 commented on Sep 13, 2016

      yan12125mannequin
      MannequinAuthor

      And I'd like to ask Ned: for me it's an improvement of an existing feature, so I guess it can enter the 3.6 branch?

    4. ned-deily commented on Sep 13, 2016

      @ned-deily
      Member

      It sounds like a fix but let's see the final patch first. If a core developer wants to apply it to the default branch for 3.7, we can decide whether it should go into 3.6, too.

    5. bitdancer commented on Sep 13, 2016

      @bitdancer
      Member

      The error can't be a SyntaxError, it must be a DeprecationWarning. If you can improve the deprecation warning text, I'd be in favor of calling that a fix to the original feature and put it in 3.6. The deprecation warning can even say that this will be a SyntaxError some time in the future.

    6. Vgr255 commented on Sep 13, 2016

      Vgr255mannequin
      Mannequin

      Hello, and thanks! I'll work on a patch this week, or at most next week. I will make it so that it's completely uncontroversial to apply it to 3.6 as well (won't change the actual feature, only prettify the error message), so no need to worry about that :)

    7. removed
      stdlibStandard Library Python modules in the Lib/ directory
      on Sep 13, 2016
    8. self-assigned this
      on Sep 13, 2016
    9. vadmium commented on Sep 14, 2016

      @vadmium
      Member

      See also bpo-28028. Serhiy suggested translating warnings to SyntaxWarning in general. Looks like that may help narrowing down the location of escaping problems.

    10. Vgr255 commented on Sep 15, 2016

      Vgr255mannequin
      Mannequin

      Besides converting the DeprecationWarning to a Syntax{Error,Warning}, I don't see an easy way to include the offending line (or even file). The place in the code where the strings are created has no idea *where* they are being defined. AIUI, either we special-case this, or we resolve bpo-28028 (but I don't think the latter can go in 3.6).

    11. removed their assignment
      on Sep 15, 2016
    12. bitdancer commented on Sep 15, 2016

      @bitdancer
      Member

      Are SyntaxWarnings silent by default? If not it can't even go into 3.7.

    13. 38 remaining items

    14. ericvsmith commented on Oct 31, 2016

      @ericvsmith
      Member

      I've pushed this to the default branch. I'll watch the buildbots.

      Then Ned can decide if this goes in to 3.6.

    15. serhiy-storchaka commented on Oct 31, 2016

      @serhiy-storchaka
      Member

      Searching on GitHub it seems to me that the most frequent issue with supporting Python 3.6 is eliminating or silencing warnings about invalid escape sequences. Any help with this is very important.

    16. Vgr255 commented on Oct 31, 2016

      Vgr255mannequin
      Mannequin

      As Nick pointed out in an earlier message on this thread and as Serhiy observed on GitHub issues, backporting this patch to 3.6 is a must. Large projects' use of Python 3.6 has shown that it's hard to track down the actual cause of the error; it only makes sense to improve that before the final release.

      Serhiy, are you working on bpo-28028 alongside this, or can it be removed from the dependencies?

    17. ericvsmith commented on Oct 31, 2016

      @ericvsmith
      Member

      I agree it would be nice to get this in to 3.6. I'm not sure I'd go so far as to say it's a must and can't wait for 3.6.1. It's a non-trivial change, and it's up to Ned to say if it can go in to 3.6.

      If you don't run with -Wall or -Werror, then you won't notice any new behavior with invalid escapes, correct?

      Maybe post to python-dev and see if we can get more reviewers?

    18. ned-deily commented on Oct 31, 2016

      @ned-deily
      Member

      I agree that the current behavior for 3.6 is very user-unfriendly so I think the risks of making such an extensive change at this point in the release cycle are outweighed by the severity of the problem. So let's get it into 3.6 now; there's still time for it to make 360b3.

    19. Vgr255 commented on Oct 31, 2016

      Vgr255mannequin
      Mannequin

      Even better than what I was aiming for :)

    20. timgraham commented on Oct 31, 2016

      timgrahammannequin
      Mannequin

      The patch is working well to identify warnings when running Django's test suite. Thanks!

    21. ericvsmith commented on Oct 31, 2016

      @ericvsmith
      Member

      I'll work on this as soon as I can, coordinating with Ned.

    22. yan12125 commented on Oct 31, 2016

      yan12125mannequin
      MannequinAuthor

      The error message is much better now, thanks you all!

      Seems the ^ pointer is not always correct. For example, in the function scope it's correct:

      $ cat test.py 
      def foo():
          s = 'C:\Program Files\Microsoft'
      
      $ python3.7 -W error test.py
        File "test.py", line 2
          s = 'C:\Program Files\Microsoft'
                 ^
      SyntaxError: invalid escape sequence \P

      On the other hand, top-level literals confuses the pointer:

      $ cat test.py               
      s = 'C:\Program Files\Microsoft'
      
      $ python3.7 -W error test.py
        File "test.py", line 1
          s = 'C:\Program Files\Microsoft'
             ^
      SyntaxError: invalid escape sequence \P

      Is that expected?

      Using 259745f9a1e4 on Arch Linux 64-bit

    23. python-dev commented on Oct 31, 2016

      python-devmannequin
      Mannequin

      New changeset ee82266ad35b by Eric V. Smith in branch '3.6':
      bpo-28128: Print out better error/warning messages for invalid string escapes. Backport to 3.6.
      https://hg.python.org/cpython/rev/ee82266ad35b

      New changeset 7aa001a48120 by Eric V. Smith in branch 'default':
      bpo-28128: Null merge with 3.6: already applied to default.
      https://hg.python.org/cpython/rev/7aa001a48120

    24. ericvsmith commented on Oct 31, 2016

      @ericvsmith
      Member

      Chi Hsuan Yen:

      I'll investigate, and open another issue as needed.

    25. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions