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

Add -3 warnings for codec convenience method changes #63742

Closed
ncoghlan opened this issue Nov 10, 2013 · 19 comments
Closed

Add -3 warnings for codec convenience method changes #63742

ncoghlan opened this issue Nov 10, 2013 · 19 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

BPO 19543
Nosy @malemburg, @brettcannon, @ncoghlan, @benjaminp, @vadmium, @serhiy-storchaka
Files
  • issue19543.patch: issue19543.patch
  • issue19543_blacklist_transforms_py27.patch
  • issue19543_unicode_decode.patch
  • 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 = None
    closed_at = <Date 2016-02-12.02:55:33.699>
    created_at = <Date 2013-11-10.09:20:47.243>
    labels = ['interpreter-core', 'type-feature']
    title = 'Add -3 warnings for codec convenience method changes'
    updated_at = <Date 2016-02-12.02:55:33.698>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2016-02-12.02:55:33.698>
    actor = 'ncoghlan'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-02-12.02:55:33.699>
    closer = 'ncoghlan'
    components = ['Interpreter Core']
    creation = <Date 2013-11-10.09:20:47.243>
    creator = 'ncoghlan'
    dependencies = []
    files = ['38979', '39226', '39576']
    hgrepos = []
    issue_num = 19543
    keywords = ['patch']
    message_count = 19.0
    messages = ['202512', '202519', '202520', '202527', '240830', '241996', '242122', '242188', '243053', '244456', '244458', '244547', '244551', '244554', '244555', '244570', '255832', '260105', '260155']
    nosy_count = 8.0
    nosy_names = ['lemburg', 'brett.cannon', 'ncoghlan', 'djmitche', 'benjamin.peterson', 'python-dev', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19543'
    versions = ['Python 2.7']

    @ncoghlan
    Copy link
    Contributor Author

    The long discussion in bpo-7475 and some subsequent discussions I had with Armin Ronacher have made it clear to me that the key distinction between the codec systems in Python 2 and Python 3 is the following differences in type signatures of various operations:

    Python 2 (8 bit str):

    codecs module: object <-> object
    convenience methods: basestring <-> basestring
    available codecs: unicode <-> str, str <-> str, unicode <-> unicode
    

    Python 3 (Unicode str):

    codecs module: object <-> object
    convenience methods: str <-> bytes
    available codecs: str <-> bytes, bytes <-> bytes, str <-> str
    

    The significant distinction is the fact that, in Python 2, the convenience methods covered all standard library codecs, but for Python 3, the codecs module needs to be used directly for the bytes <-> bytes codecs and the one str <-> str codec (since those codecs no longer satisfy the constraints of the text model related convenience methods).

    After attempting to implement a 2to3 fixer for these non-Unicode codecs in bpo-17823, I realised that wouldn't really work properly (since it's a data driven error based on the behaviour of the named codec), so I'm rejecting that proposal and replacing it with this one for additional Py3k warnings in Python 2.7.7.

    My proposal is to take the following cases and make them produce warnings under Python 2.7.7 when Py3k warnings are enabled (remember, these are the 2.7 types, not the 3.x ones):

    • the str.encode method is called (redirect to codecs.encode to handle arbitrary input types in a forward compatible way)

    • the unicode.decode method is called (redirect to codecs.decode to handle arbitrary input types)

    • PyUnicode_AsEncodedString produces something other than an 8-bit string (redirect to codecs.encode for arbitrary output types)

    • PyUnicode_Decode produces something other than a unicode string (redirect to codecs.decode for arbitrary output types)

    For the latter two cases, bpo-17828 includes updates to the Python 3 error messages to similarly redirect to the convenience functions in the codecs module. However, the removed convenience methods will continue to simply trigger AttributeError in Python 3 with no special casing.

    @ncoghlan ncoghlan added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Nov 10, 2013
    @vadmium
    Copy link
    Member

    vadmium commented Nov 10, 2013

    Just thinking the first case might get quite a few false positives. Maybe that would still be acceptable, I dunno.

    • the str.encode method is called (redirect to codecs.encode to handle arbitrary input types in a forward compatible way)

    I guess you are trying to catch cases like this, which I have come across quite a few times:

    data.encode("hex")  # data is a byte string

    But I think you would also catch cases that depend on Python 2 “str” objects automatically converting to Unicode. Here are some examples taken from real code:

    file_name.encode("utf-8")  # File name parameter may be str or unicode

    # Code meant to be compatible with both Python 2 and 3:
    """<?xml . . . encoding="iso-8859-1"?>""".encode("iso-8859-1")
    ("data %s\n" % len(...)).encode("ascii")

    @malemburg
    Copy link
    Member

    On 10.11.2013 10:20, Nick Coghlan wrote:

    The long discussion in bpo-7475 and some subsequent discussions I had with Armin Ronacher have made it clear to me that the key distinction between the codec systems in Python 2 and Python 3 is the following differences in type signatures of various operations:

    Python 2 (8 bit str):

    codecs module: object \<-\> object
    convenience methods: basestring \<-\> basestring
    available codecs: unicode \<-\> str, str \<-\> str, unicode \<-\> unicode
    

    Python 3 (Unicode str):

    codecs module: object \<-\> object
    convenience methods: str \<-\> bytes
    available codecs: str \<-\> bytes, bytes \<-\> bytes, str \<-\> str
    

    The significant distinction is the fact that, in Python 2, the convenience methods covered all standard library codecs, but for Python 3, the codecs module needs to be used directly for the bytes <-> bytes codecs and the one str <-> str codec (since those codecs no longer satisfy the constraints of the text model related convenience methods).

    Please remember that the codec sub-system is extensible. It's
    easily possible to add more codecs via registered codec
    search functions.

    Whatever you add as warning has to be aware of the fact that
    there may be codecs in the system that are not part of the
    stdlib and which can potentially implement codecs that use
    other type combinations that the ones you listed above.

    @ncoghlan
    Copy link
    Contributor Author

    Martin: you're right, it wouldn't be feasible to check for the 8-bit str encoding case, since the types of string literals will implicitly change between the two versions. However, the latter three cases would be feasible to check (the unicode.decode one is particularly pernicious, since it's the culprit that can lead to UnicodeEncodeErrors on a decoding operation as Python implicitly tries to encode a Unicode string as ASCII).

    MAL: The latter two Py3k warnings would be in the same place as the corresponding output type errors in Python 3 (i.e. all in unicodeobject.c), so they would never trigger for the general codecs machinery.

    Python 2 actually already has output type checks in the same place as the proposed warnings, it just only checks for "basestring" rather than anything more specific. Those two warnings would just involve adding the more restrictive Py3k-style check when -3 was enabled.

    A Py3k warning for unicode.decode is just a straight "this method won't be there any more in Python 3" warning, since there's no way for the conversion from Python 2 to Python 3 to implicitly replace a Unicode string with 8-bit data the way string literals switch from 8-bit data to Unicode text.

    @djmitche
    Copy link
    Mannequin

    djmitche mannequin commented Apr 14, 2015

    This fixes the four cases Nick referred to, although not in the functions expected:

    • str.encode no longer exists
    • unicode.decode no longer exists
    • encoders used by str.encode must return bytes (does not apply to codecs.encode)
    • decoders used by unicode.decode must return unicode (does not apply to codecs.decode)

    @ncoghlan
    Copy link
    Contributor Author

    For the warnings, it's actually bytes.decode() (et al) that are expected to return str, and str.encode() that's expected to return bytes. The codecs themselves remain free to do what they want (hence the recommendation to use codecs.encode() and codecs.decode() to invoke arbitrary codecs without the type constraints of the builtins). Aside from that, those 2 warnings and the unicode.decode() one look good.

    However, the "bytes.encode()" warning is the one we determined couldn't be treated as a warning as "text".encode("ascii") is valid in both Python 2 & 3, it just does a str->str conversion in 2.x, and a str -> bytes conversion in 3.x.

    @serhiy-storchaka
    Copy link
    Member

    I think we should just backport bpo-19619 and bpo-20404.

    @serhiy-storchaka
    Copy link
    Member

    Here is a patch that backports bpo-19619 and bpo-20404 with changing an exception to Py3k warning, and makes necessary changes in other modules and tests.

    $ ./python -3
    Python 2.7.10rc0 (2.7:4234b0dd2a54+, Apr 28 2015, 16:51:51) 
    [GCC 4.8.2] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    >>> 'abcd'.decode('hex')
    __main__:1: DeprecationWarning: 'hex' is not a text encoding; use codecs.decode() to handle arbitrary codecs
    '\xab\xcd'

    @serhiy-storchaka
    Copy link
    Member

    What would you say about this Benjamin?

    @serhiy-storchaka
    Copy link
    Member

    Nick, Benjamin, could you please look at the patch?

    @ncoghlan
    Copy link
    Contributor Author

    Serhiy's patch looks to me like it would pragmatically cover all the cases most likely to affect porting efforts: using the standard library bytes<->bytes codecs through the convenience methods.

    For a Python 2 backport, I'm slightly more concerned with exposing the argument in the constructor signature, but see value in being consistent with Python 3 if anyone decides to use this to check a custom codec.

    The main advantage this approach has over the typecheck based approach is that it can correctly warn about "data".encode("hex"), while a typecheck based approach can't distinguish that from "text".encode("ascii")

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 31, 2015

    New changeset cf6e782a7f94 by Serhiy Storchaka in branch '2.7':
    Issue bpo-19543: Emit deprecation warning for known non-text encodings.
    https://hg.python.org/cpython/rev/cf6e782a7f94

    @serhiy-storchaka
    Copy link
    Member

    Committed patch covers a large part of this issue, but not all.

    Following patch emits py3k warning for unicode.decode(). For now unicode(u'a', 'ascii') is forbidden, but u'a'.decode('ascii') is allowed in 2.7.

    The risk of false positive in this patch is lower than in emitting warning on str.encode(), but is larger than in just committed patch.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 31, 2015

    New changeset 0347f6e14ad6 by Tal Einat in branch '3.5':
    Issue bpo-19543: Implementation of isclose as per PEP-485
    https://hg.python.org/cpython/rev/0347f6e14ad6

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 31, 2015

    New changeset 5c8c123943cf by Tal Einat in branch 'default':
    Issue bpo-19543: Implementation of isclose as per PEP-485
    https://hg.python.org/cpython/rev/5c8c123943cf

    @ncoghlan
    Copy link
    Contributor Author

    ncoghlan commented Jun 1, 2015

    The last two commit notifications were intended for issue bpo-24270.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Dec 3, 2015

    New changeset c89a0f24d5f6 by Serhiy Storchaka in branch '2.7':
    Issue bpo-19543: Added Py3k warning for decoding unicode.
    https://hg.python.org/cpython/rev/c89a0f24d5f6

    @serhiy-storchaka
    Copy link
    Member

    Is there something left to do with this issue?

    @ncoghlan
    Copy link
    Contributor Author

    I think so - if anyone spots another place a Py3k warning could be usefully emitted, it can be handled as a new issue.

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants