gh-129349: Accept bytes in bytes.fromhex()/bytearray.fromhex() by lordmauve · Pull Request #129844 · python/cpython · GitHub

Change bytes.fromhex() and bytearray.fromhex() to accept a bytes object interpreted as ASCII.

This matches the behaviour of int e.g

>>> int(b'123abc', 16)
1194684
>>> bytes.fromhex(b'123abc')
b'\x12:\xbc'

Fixes #129349

Issue: bytes.fromhex() should parse a bytes #129349

We need more tests where invalid non-hexadecimal bytes are given (e.g., b"à", '\uD834\uDD1E'.encode() or bytes with NULs) and where non-bytes/str/bytearray objects are also passed.

cc @vstinner

Although the use case of git cat-file --batch is relevant, I still think it's a little too niche for this. I think the output of git cat-file --batch should be post-processed (and you should indeed call .decode() instead) but others might disagree.

I'm sorry I reviewed the PR before thinking more about this. Actually, we have binascii.unhexlify that already supports bytes inputs. And the docs say:

Similar functionality (accepting only text string arguments, but more liberal towards whitespace) is also accessible using the bytes.fromhex() class method.

So I think the text-only feature for bytes.fromhex was actually desired.
I also tried to search on GitHub whether there are use cases for this, but I couldn't find any that actually used a non-string input. So I think this modification is likely not ideal. We might want to really restrict the input type for built-in types and perhaps be less restrictive with binascii.

Thus, sorry for having reviewed the PR (which seemed I endorsed the change), but I don't think we need to make that change. For your use-case, I think you can definitely use binascii.unhexlify instead (for instance binascii.unhexlify(b'012abc') would return b'\x01\x2a\xbc as you tested).

Note that binascii actually supports any object implementing the buffer protocol, so it's likely the one that needs to be used in such cases (assuming that whitespaces are cleaned). See https://github.com/python/cpython/blob/main/Python/pystrhex.c.

TL;DR: I'm rather -0.5 for the inclusion of this feature but if others disagree and find it good to align it with binascii.unhexlify, then I won't be against.

cc @serhiy-storchaka @vstinner

Note: The PR you wrote is of good quality for a first contribution. It's just that the feature can be quite niche, especially if it's added on a built-in class.

binascii seems a little obscure given that int(bytes, 16) works and binascii docs say:

Normally, you will not use these functions directly but use wrapper modules like base64

It seems like a logical extension that bytes.fromhex() should support bytes and it still leaves a place for binascii as the version that supports the buffer protocol.

It's not my first credited contribution, just the first under my own GitHub account maybe.

It's not my first credited contribution, just the first under my own GitHub account maybe.

Sorry I got confused with another account (but the fact that the PR is good remains).

It seems like a logical extension that bytes.fromhex() should support bytes and it still leaves a place for binascii as the version that supports the buffer protocol.

Yes, but OTOH, it's a built-in and except for your use case, I couldn't find other use cases where binascii.unhexlify couldn't fit the bill (or just a call to .decode() before). So I would defer the final decision to Victor and/or Serhiy on that matter.

LGTM

Maybe add also a test for memoryview or array.array?

Maybe add also a test for memoryview or array.array?

Ok, added this and support for the PyBuf_SIMPLE buffer protocol.

Question: are bytes also bytes-like objects? if so, you can just link the term. Or more generally, isn't it objects that support the buffer protocol?

Yes, but it seems less accessible to users to just link bytes-like object because that dives into a description of the buffer protocol which is lower level than the audience I was writing for in builtins docs.

Maybe the fault is with :term:\bytes-like object`` because it could just list types that duck-type like bytes.

So I hedged and did both.

You could sa "that support the buffer protocol such as memoryviews" and add a link to whatever example of a type that supports the buffer protocol you used

I prefer to say "bytes and bytes-like", it's more explicit.

Oh no, there is now a conflict on clinic/ files. You can merge main into your branch and re-run make clinic.

Fixes python#129349

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Merged, thank you @lordmauve!

bedevere-app Bot added the awaiting review label Feb 8, 2025

bedevere-app Bot mentioned this pull request Feb 8, 2025

bytes.fromhex() should parse a bytes #129349

Closed

picnixz reviewed Feb 8, 2025

View reviewed changes

Comment thread Doc/library/stdtypes.rst Outdated

Comment thread Doc/library/stdtypes.rst Outdated

Comment thread Lib/test/test_bytes.py Outdated

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-02-08-09-55-33.gh-issue-129349.PkcG-l.rst Outdated

Comment thread Objects/bytesobject.c Outdated

lordmauve added a commit to lordmauve/cpython that referenced this pull request Feb 9, 2025

Apply suggestions from @picnixz in python#129844

ad5cc10

picnixz reviewed Feb 9, 2025

View reviewed changes

vstinner approved these changes Feb 20, 2025

View reviewed changes

Comment thread Objects/bytesobject.c

bedevere-app Bot added awaiting merge and removed awaiting review labels Feb 20, 2025

vstinner reviewed Feb 26, 2025

View reviewed changes

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-02-08-09-55-33.gh-issue-129349.PkcG-l.rst Outdated

serhiy-storchaka approved these changes Feb 27, 2025

View reviewed changes

Comment thread Doc/library/stdtypes.rst Outdated

Comment thread Doc/library/stdtypes.rst Outdated

Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-02-08-09-55-33.gh-issue-129349.PkcG-l.rst Outdated

lordmauve added a commit to lordmauve/cpython that referenced this pull request Mar 7, 2025

Apply suggestions from @picnixz in python#129844

f771a5e

lordmauve force-pushed the lordmauve/issue129349 branch from ad5cc10 to 1a18438 Compare March 7, 2025 11:12

vstinner reviewed Mar 7, 2025

View reviewed changes

Comment thread Doc/library/stdtypes.rst

picnixz reviewed Mar 7, 2025

View reviewed changes

vstinner reviewed Mar 8, 2025

View reviewed changes

Comment thread Doc/library/stdtypes.rst Outdated

vstinner reviewed Mar 8, 2025

View reviewed changes

Comment thread Objects/bytesobject.c Outdated

lordmauve force-pushed the lordmauve/issue129349 branch from 91566c9 to ba05050 Compare March 9, 2025 12:51

lordmauve added a commit to lordmauve/cpython that referenced this pull request Mar 10, 2025

Apply suggestions from @picnixz in python#129844

82c35f0

lordmauve force-pushed the lordmauve/issue129349 branch from be4b9a4 to 1e035db Compare March 10, 2025 20:07

lordmauve added 5 commits March 12, 2025 09:43

Accept bytes in bytes.fromhex()/bytearray.fromhex()

81d7ea2

Fixes python#129349

Update documentation

ea02542

Apply suggestions from @picnixz in python#129844

76d28ce

Fix sphinx-lint

cff091c

Mention bytearray in docs and blurb

be92188

lordmauve and others added 5 commits March 12, 2025 09:43

Use buffer protocol to support all byte-like objects

b0e3eb5

Insert missing braces

ef92b4e

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

Update Doc/library/stdtypes.rst

685f583

Add test for invalid type in bytes.fromhex()

f859c3c

PEP-7

31b9ab0

lordmauve force-pushed the lordmauve/issue129349 branch from 1e035db to 31b9ab0 Compare March 12, 2025 09:45

vstinner merged commit e0637ce into python:main Mar 12, 2025

bedevere-app Bot removed the awaiting merge label Mar 12, 2025

lordmauve deleted the lordmauve/issue129349 branch March 12, 2025 12:54

                 (Contrubuted by Sergey B Kirpichev in :gh:`87790`.)
+              * The :func:`bytes.fromhex` and :func:`bytearray.fromhex` methods now accept
+                ASCII :class:`bytes` and :term:`bytes-like objects <bytes-like object>`.

gh-129349: Accept bytes in bytes.fromhex()/bytearray.fromhex() by lordmauve · Pull Request #129844 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-129349: Accept bytes in bytes.fromhex()/bytearray.fromhex() - #129844

gh-129349: Accept bytes in bytes.fromhex()/bytearray.fromhex()#129844vstinner merged 10 commits intopython:mainpython/cpython:mainfrom lordmauve:lordmauve/issue129349lordmauve/cpython:lordmauve/issue129349Copy head branch name to clipboard

Conversation

lordmauve commented Feb 8, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

picnixz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

picnixz commented Feb 8, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

picnixz commented Feb 9, 2025

Uh oh!

lordmauve commented Feb 9, 2025

Uh oh!

picnixz commented Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lordmauve commented Mar 7, 2025

Uh oh!

Uh oh!

picnixz Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

lordmauve Mar 7, 2025

Choose a reason for hiding this comment

Uh oh!

picnixz Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner Mar 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner commented Mar 12, 2025

Uh oh!

vstinner commented Mar 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-129349: Accept bytes in bytes.fromhex()/bytearray.fromhex()#129844
vstinner merged 10 commits into
python:mainfrom
lordmauve:lordmauve/issue129349

lordmauve commented Feb 8, 2025 •
edited by bedevere-app Bot
Loading

picnixz left a comment •
edited
Loading

picnixz commented Feb 9, 2025 •
edited
Loading

picnixz Mar 7, 2025 •
edited
Loading