Skip to content

Skip invalid escape sequence check on 2.6#763

Closed
sigmavirus24 wants to merge 1 commit intoPyCQA:masterfrom
sigmavirus24:bug/755
Closed

Skip invalid escape sequence check on 2.6#763
sigmavirus24 wants to merge 1 commit intoPyCQA:masterfrom
sigmavirus24:bug/755

Conversation

@sigmavirus24
Copy link
Copy Markdown
Member

Python 2.6 has a bug (https://bugs.python.org/issue15054) that prevents
us from tokenizing strings properly that are b'' strings. This means we
cannot reliably detect future deprecated escape sequences on 2.6

Closes gh-755

@sigmavirus24
Copy link
Copy Markdown
Member Author

sigmavirus24 commented Apr 25, 2018

cc @jwilk

Comment thread pycodestyle.py Outdated
Okay: regex = r'\.png$'
W605: regex = '\.png$'
"""
if (2, 5) < sys.version_info <= (2, 6):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition is off-by-one: it's true in Python 2.5 (which doesn't have b'' strings, so not affected by this bug), but false in Python 2.6. How about:

if sys.version_info[:2] == (2, 6):

?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've updated the range to be correct.

Python 2.6 has a bug (https://bugs.python.org/issue15054) that prevents
us from tokenizing strings properly that are b'' strings. This means we
cannot reliably detect future deprecated escape sequences on 2.6

Closes PyCQAgh-755
@jones77
Copy link
Copy Markdown

jones77 commented May 30, 2018

Guessing that a unit test needs updating?

py26 runtests: commands[1] | /home/travis/build/PyCQA/pycodestyle/.tox/py26/bin/python pycodestyle.py --testsuite testsuite
testsuite/W60.py:16:1: error W605 not found
testsuite/W60.py:18:1: error W605 not found
3665 lines tested: 38 files, 405 test cases, 2 failures.
Test failed.
ERROR: InvocationError: '/home/travis/build/PyCQA/pycodestyle/.tox/py26/bin/python pycodestyle.py --testsuite testsuite'
___________________________________ summary ____________________________________
ERROR:   py26: commands failed```

@jones77
Copy link
Copy Markdown

jones77 commented May 30, 2018

FWIW I can reproduce this when using Python2 and trying to escape square brackets:

WW211     ERROR_LOG_PATTERN = re.compile(
  212         '(?P<date>[^ ]+)'
  213         ' (?P<time>[^ ]+)'
  214         ' \[(?P<error_type>.*)\]'

211 col 7 warning| W605 invalid escape sequence '\[' [pep8]

@sigmavirus24
Copy link
Copy Markdown
Member Author

FWIW I can reproduce this when using Python2 and trying to escape square brackets:

You should absolutely see this on 2.7. It's not that it's a problem on 2.7 but if your code lives past 2020, you should be well-warned that it will break on the latest versions of 3.7 and there are things you can do today to avoid that.

@jones77
Copy link
Copy Markdown

jones77 commented May 30, 2018

Thanks, though I still feel like I'm missing something 'cos the docs / internet search seems to suggest escaping square brackets is ok.

https://docs.python.org/3/library/re.html

To match a literal ']' inside a set, precede it with a backslash, or place it at the beginning of the set. For example, both [()[\]{}] and []()[{}] will both match a parenthesis.

Anyhoo, gonna use [[] and []] instead.

@sigmavirus24
Copy link
Copy Markdown
Member Author

\[ isn't a valid escape sequence. You can also use r'\['.

@jones77
Copy link
Copy Markdown

jones77 commented May 31, 2018

Thanks again, found the pbk&c, :-S the following are different.

ERROR_LOG_PATTERN_GOOD = re.compile(
    '(?P<date>[^ ]+)'
    ' (?P<time>[^ ]+)'
    r' \[(?P<error_type>.*)\]'
)

ERROR_LOG_PATTERN_BAD = re.compile(
    r'(?P<date>[^ ]+)'
    ' (?P<time>[^ ]+)'
    ' \[(?P<error_type>.*)\]'
)

@sigmavirus24
Copy link
Copy Markdown
Member Author

@jones77 I'm not certain if you're looking for an explanation or if you're merely commenting on the ability to use r prefixed strings. Either way, I think that discussion is better suited to a different place than this pull request at this point.

@jdufresne
Copy link
Copy Markdown
Contributor

Given the comments in #720, Python 2.6 support will be dropped. So perhaps this PR is no longer required?

@sigmavirus24 sigmavirus24 deleted the bug/755 branch June 4, 2018 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants