FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent by Vgr255 · Pull Request #5849 · python/cpython · GitHub

/ cpython Public

bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent - #5849

Closed
Vgr255 wants to merge 1 commit into
python:masterfrom
Vgr255:issue32912
ConversationCommits (1)ChecksFiles changed
Closed

bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent#5849
Vgr255 wants to merge 1 commit into
python:masterfrom
Vgr255:issue32912

Conversation

Vgr255 commented Feb 24, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

What the title says. Properly tested and ready for 3.8!

https://bugs.python.org/issue32912

:EDIT:

I don't know how to add a NEWS entry, but I have to go for now. I'll look into that tomorrow.

serhiy-storchaka reviewed Feb 24, 2018
View reviewed changes
Comment thread Python/ast.c
NULL, NULL) < 0)
{
if (PyErr_ExceptionMatches(PyExc_DeprecationWarning)) {
if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) {

serhiy-storchaka Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

See the comment below.

Vgr255 Feb 24, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, the end goal here is to get a SyntaxError, however this is not yet possible as we have to wait for two releases before this can happen, so we can't yet convert it to a SyntaxError. I probably should update that comment, though (I'll do that tomorrow when I get home).

Comment thread Objects/unicodeobject.c
return NULL;
if (first_invalid_escape != NULL) {
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
if (PyErr_WarnFormat(PyExc_SyntaxWarning, 1,

serhiy-storchaka Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is not related to syntax.

Vgr255 Feb 24, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This warning was added back in 3.6 when we started deprecating the invalid escapes. In 3.8, we're upgrading these to SyntaxWarning so that they're no longer silent by default, and it will eventually be a SyntaxError. The end goal here is to raise a SyntaxError for this use, but this will have to wait for two more releases.

serhiy-storchaka Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This warning is emitted by the "unicode-escape" codec.

>>> b'\\z'.decode('unicode-escape')
__main__:1: DeprecationWarning: invalid escape sequence '\z'
'\\z'

Vgr255 Feb 24, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, and this warning was added as part of bpo-27364 to keep consistency with the rest of the language. It only makes sense to upgrade this warning along with everything else, or else this codec falls out of sync with the rest of the language.

Comment thread Objects/bytesobject.c
return NULL;
if (first_invalid_escape != NULL) {
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
if (PyErr_WarnFormat(PyExc_SyntaxWarning, 1,

serhiy-storchaka Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is not related to syntax.

Vgr255 Feb 24, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This warning was added back in 3.6 when we started deprecating the invalid escapes. In 3.8, we're upgrading these to SyntaxWarning so that they're no longer silent by default, and it will eventually be a SyntaxError. The end goal here is to raise a SyntaxError for this use, but this will have to wait for two more releases.

serhiy-storchaka Feb 24, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No, this warning is emitted by codecs.escape_decode() (which is used in pickle).

>>> codecs.escape_decode(b'\\z')
__main__:1: DeprecationWarning: invalid escape sequence '\z'
(b'\\z', 2)

Vgr255 Feb 24, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, and this warning was added as part of bpo-27364 to keep consistency with the rest of the language. It only makes sense to upgrade this warning along with everything else, or else the codecs module falls out of sync with the rest of the language.

serhiy-storchaka requested changes Mar 27, 2018
View reviewed changes

serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The codecs module shouldn't raise SyntaxWarning or SyntaxError. It should raise DeprecationWarning, which later will be converted to UnicodeDecodeError.

Vgr255 commented Mar 27, 2018

Copy link
Copy Markdown
Contributor Author

I haven't had the time to work on this, sorry! I'll get back to it hopefully sometime soon.

tirkarthi commented May 16, 2019

Copy link
Copy Markdown
Member

I think this was done with #9652 and the bpo issue is also marked as resolved.

Vgr255 mannequin mentioned this pull request Nov 2, 2022
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

serhiy-storchaka serhiy-storchaka requested changes

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL