bytes: support keyword arguments in hex() by devyubin · Pull Request #8312 · RustPython/RustPython

devyubin · 2026-07-18T05:34:43Z

This PR follows our AI policy

Summary

bytes/bytearray/memoryview's hex() rejected its sep and bytes_per_sep
arguments when passed by keyword, even though CPython accepts them either
positionally or by keyword.

b'\xb9\x01\xef'.hex(sep=':', bytes_per_sep=2)
# before: TypeError: Unexpected keyword argument sep
# after:  'b9:01ef'   # matches CPython

Cause

hex() took its arguments as bare OptionalArg<T> method parameters, whose
FromArgs only consumes positionals (take_positional()) and never keywords.
The argument names were never registered, so sep= / bytes_per_sep= stayed in
kwargs and tripped check_kwargs_empty.

Fix

Move the arguments into a #[derive(FromArgs)] struct (ByteInnerHexOptions)
with #[pyarg(any, optional)] fields, which generates take_positional_keyword
so both positional and keyword forms are accepted — matching CPython's
positional-or-keyword signature (no / or * in its Argument Clinic definition).
Applied consistently to bytes, bytearray, and memoryview; the computation
still delegates to the existing inner.hex(...), so the error cases are preserved
(None → TypeError, separator len != 1 → ValueError). Re-enables
test_hex_separator_basics by removing its expectedFailure marker.

Test

Verified locally:

cargo run -- -m test test_bytes -v → Tests result: SUCCESS (Ran 317 tests, OK (skipped=15, expected failures=18))
- test_hex_separator_basics (BytesTest / ByteArrayTest) ... ok
- test_hex_separator_five_bytes / test_hex_separator_six_bytes / test_hex ... ok

Assisted-by: Claude Code:claude-opus-4-8

Summary by CodeRabbit

Improvements
- Standardized hexadecimal formatting options across bytes, bytearray, and memoryview.
- Consistent support for custom separators and configurable bytes-per-group formatting.
- Existing .hex() behavior remains unchanged while argument handling is more consistent across these types.

coderabbitai · 2026-07-18T05:35:04Z

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

Lib/test/test_memoryview.py is excluded by !Lib/**

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: d64fbc95-3569-4574-bcf4-88c7e7eaa63b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a shared ByteInnerHexOptions argument struct and updates bytes, bytearray, and memoryview hex methods to use it.

Changes

Hex options integration

Layer / File(s)	Summary
Shared hex argument contract `crates/vm/src/bytes_inner.rs`	Adds a derived options struct containing optional separator and grouping arguments.
Builtin hex method wiring `crates/vm/src/builtins/bytes.rs`, `crates/vm/src/builtins/bytearray.rs`, `crates/vm/src/builtins/memory.rs`	Updates the three hex methods to accept, destructure, and forward `ByteInnerHexOptions`.
Estimated code review effort: 2 (Simple)	~10 minutes

Suggested reviewers: youknowone, shaharnaveh

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: enabling keyword arguments for hex() across bytes-related types.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-18T05:35:36Z

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[ ] test: cpython/Lib/test/test_array.py (TODO: 55)

dependencies:

dependent tests: (102 tests)

[x] lib: cpython/Lib/zoneinfo
[x] test: cpython/Lib/test/test_zoneinfo (TODO: 6)

dependencies:

zoneinfo

dependent tests: (2 tests)

zoneinfo: test_plistlib test_zoneinfo

[x] test: cpython/Lib/test/test_list.py (TODO: 4)
[x] test: cpython/Lib/test/test_listcomps.py (TODO: 1)
[ ] test: cpython/Lib/test/test_userlist.py (TODO: 1)

dependencies:

dependent tests: (no tests depend on list)

[x] test: cpython/Lib/test/test_module (TODO: 2)

dependencies:

dependent tests: (no tests depend on module)

[ ] test: cpython/Lib/test/test_set.py (TODO: 6)

dependencies:

dependent tests: (no tests depend on set)

[x] test: cpython/Lib/test/test_resource.py (TODO: 4)

dependencies:

dependent tests: (4 tests)

resource: test_os test_selectors test_subprocess test_threading

[x] lib: cpython/Lib/re
[x] lib: cpython/Lib/sre_compile.py
[x] lib: cpython/Lib/sre_constants.py
[x] lib: cpython/Lib/sre_parse.py
[ ] test: cpython/Lib/test/test_re.py (TODO: 12)
[x] test: cpython/Lib/test/re_tests.py

dependencies:

re

dependent tests: (81 tests)

re: test_android test_ast test_asyncio test_binascii test_builtin test_bytes test_clinic test_cmd_line test_ctypes test_dict test_dis test_docxmlrpc test_dtrace test_email test_embed test_faulthandler test_filecmp test_fileinput test_fnmatch test_format test_frame test_fstring test_functools test_future_stmt test_generated_cases test_genericalias test_glob test_hashlib test_http_cookiejar test_httplib test_httpservers test_imaplib test_importlib test_inspect test_ipaddress test_launcher test_logging test_mailbox test_mmap test_optparse test_ordered_dict test_platform test_pprint test_pydoc test_pyexpat test_pyrepl test_re test_regrtest test_runpy test_set test_site test_smtplib test_socket test_ssl test_strftime test_strptime test_strtod test_structseq test_symtable test_syntax test_sys test_sysconfig test_tarfile test_tempfile test_tokenize test_tools test_traceback test_turtle test_typing test_unittest test_unparse test_venv test_webbrowser test_winapi test_with test_wsgiref test_xmlrpc test_zipfile test_zipimport test_zoneinfo test_zstd

[ ] test: cpython/Lib/test/test_memoryview.py (TODO: 8)

dependencies:

dependent tests: (no tests depend on memoryview)

[x] lib: cpython/Lib/weakref.py
[x] lib: cpython/Lib/_weakrefset.py
[x] test: cpython/Lib/test/test_weakref.py (TODO: 13)
[x] test: cpython/Lib/test/test_weakset.py

dependencies:

weakref

dependent tests: (222 tests)

[x] lib: cpython/Lib/io.py
[x] lib: cpython/Lib/_pyio.py
[ ] test: cpython/Lib/test/test_io.py (TODO: 13)
[x] test: cpython/Lib/test/test_bufio.py
[x] test: cpython/Lib/test/test_fileio.py (TODO: 1)
[ ] test: cpython/Lib/test/test_memoryio.py (TODO: 27)

dependencies:

io

dependent tests: (108 tests)

io: test__colorize test_android test_argparse test_ast test_asyncio test_base64 test_buffer test_bufio test_builtin test_bz2 test_calendar test_cmd test_cmd_line_script test_codecs test_compile test_compileall test_compiler_assemble test_concurrent_futures test_configparser test_contextlib test_csv test_dbm_dumb test_descr test_dis test_email test_enum test_file test_fileinput test_fileio test_ftplib test_generated_cases test_getpass test_gzip test_hashlib test_http_cookiejar test_httplib test_httpservers test_importlib test_inspect test_io test_json test_largefile test_logging test_lzma test_mailbox test_marshal test_memoryio test_memoryview test_mimetypes test_minidom test_multibytecodec test_optparse test_pathlib test_pdb test_peg_generator test_pickle test_pickletools test_platform test_plistlib test_pprint test_print test_profile test_pstats test_pty test_pulldom test_pydoc test_pyexpat test_pyrepl test_quopri test_regrtest test_robotparser test_sax test_shlex test_shutil test_site test_smtplib test_socket test_socketserver test_subprocess test_support test_sys test_tarfile test_tempfile test_threadedtempfile test_timeit test_tokenize test_traceback test_types test_typing test_unittest test_univnewlines test_urllib test_urllib2 test_uuid test_wave test_webbrowser test_winconsoleio test_wsgiref test_xml_dom_xmlbuilder test_xml_etree test_xml_etree_c test_xmlrpc test_xpickle test_zipapp test_zipfile test_zipimport test_zoneinfo test_zstd

[ ] test: cpython/Lib/test/test_bytes.py (TODO: 21)

dependencies:

dependent tests: (no tests depend on bytes)

[x] lib: cpython/Lib/lzma.py
[x] test: cpython/Lib/test/test_lzma.py (TODO: 11)

dependencies:

lzma

dependent tests: (101 tests)

[ ] lib: cpython/Lib/test/support
[x] test: cpython/Lib/test/test_support.py (TODO: 3)
[x] test: cpython/Lib/test/test_script_helper.py

dependencies:

support (native: main, _hashlib, _helpers, _hmac, _imp, _interpchannels, _opcode, _remote_debugging, _testcapi, _testinternalcapi, _testlimitedcapi, _thread, _winapi, asyncio.events, collections.abc, concurrent.interpreters, concurrent.interpreters._crossinterp, ctypes.wintypes, email._header_value_parser, errno, faulthandler, gc, hypothesis, hypothesis.configuration, hypothesis.database, import_helper, importlib.machinery, importlib.util, logging.handlers, math, msvcrt, os.path, os_helper, pwd, resource, script_helper, select, setuptools, setuptools._distutils, sys, time, unicodedata, urllib.error, urllib.parse, urllib.request, zlib)
- opcode (native: _opcode, builtins)
- platform (native: _wmi, itertools, java.lang, sys, vms_lib, winreg)
- sysconfig (native: _sysconfig, _winapi, importlib.machinery, importlib.util, os.path, sys)
- unittest (native: _io, _log, async_case, builtins, case, loader, main, os.path, result, runner, signals, suite, sys, time, unittest.util, util)
- warnings (native: _contextvars, _thread, _warnings, builtins, sys)
- _colorize, annotationlib, ast, bz2, codecs, collections, compression, contextlib, ctypes, dataclasses, datetime, decimal, dis, enum, functools, getopt, getpass, glob, gzip, hashlib, importlib, inspect, io, locale, logging, lzma, multiprocessing, os, pathlib, py_compile, re, selectors, shlex, shutil, signal, smtplib, socket, stat, string, struct, subprocess, tempfile, textwrap, threading, tkinter, tracemalloc, types, venv, zipfile

dependent tests: (2 tests)

support: test_pathlib test_pyrepl

[x] lib: cpython/Lib/socket.py
[ ] test: cpython/Lib/test/test_socket.py (TODO: 17)

dependencies:

socket

dependent tests: (101 tests)

[x] lib: cpython/Lib/csv.py
[x] test: cpython/Lib/test/test_csv.py (TODO: 25)

dependencies:

csv

dependent tests: (4 tests)

csv: test_csv test_genericalias
- importlib.metadata: test_importlib test_zoneinfo

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

ShaharNaveh · 2026-07-18T06:28:57Z

UNEXPECTED SUCCESS: test_memoryview_hex_separator (test.test_memoryview.OtherTest.test_memoryview_hex_separator)

yaay, can you please remove that test mark as well?

youknowone · 2026-07-19T05:08:00Z

+#[derive(FromArgs)]
+pub(crate) struct ByteInnerHexOptions {
+    #[pyarg(any, optional)]
+    pub sep: OptionalArg<Either<PyStrRef, PyBytesRef>>,


Does sep strictly allow str or bytes? Otherwise, could you please check if ArgStrOrBytesLike fits better here? ArgStrOrBytesLike allows a little bit more than bytes. So it might not fit.

Thanks for the suggestion! It was definitely worth double-checking.

sep strictly accepts only str or bytes. This means ArgStrOrBytesLike would be too permissive for this case.

Since ArgStrOrBytesLike falls back to ArgBytesLike for non-string inputs, it accepts any object that implements the buffer protocol (like bytearray, memoryview, and array). CPython explicitly rejects these for sep, so I kept Either<PyStrRef, PyBytesRef> to match CPython's strict type checking exactly.

I also unmarked test_memoryview_hex_separator as requested. I verified it locally and the test passed (test_memoryview -> SUCCESS, run=171).

Thank you so much for confiriming!

ShaharNaveh

tysm!

* bytes: support keyword arguments in hex() * bytes: unmark test_memoryview_hex_separator

bytes: support keyword arguments in hex()

20c4fcc

moreal added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 18, 2026

youknowone reviewed Jul 19, 2026

View reviewed changes

bytes: unmark test_memoryview_hex_separator

7f2670d

ShaharNaveh approved these changes Jul 19, 2026

View reviewed changes

youknowone merged commit bc12a97 into RustPython:main Jul 22, 2026
27 checks passed

coderabbitai Bot mentioned this pull request Aug 18, 2026

memoryview: count exports, hash the exporter, and index through __index__ #8553

Merged

youknowone pushed a commit that referenced this pull request Sep 16, 2026

bytes: support keyword arguments in hex() (#8312)

79f7da3

* bytes: support keyword arguments in hex() * bytes: unmark test_memoryview_hex_separator

Navigation Menu

Uh oh!

bytes: support keyword arguments in hex() - #8312

bytes: support keyword arguments in hex()#8312
youknowone merged 2 commits into
RustPython:mainfrom
devyubin:fix-bytes-hex-keyword-args

devyubin commented Jul 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

Review skipped

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

ShaharNaveh commented Jul 18, 2026

Uh oh!

youknowone Jul 19, 2026

Uh oh!

devyubin Jul 20, 2026 •

edited

Loading

Uh oh!

youknowone Jul 22, 2026

Uh oh!

ShaharNaveh left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

devyubin commented Jul 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Cause

Fix

Test

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Library Dependencies

Uh oh!

ShaharNaveh commented Jul 18, 2026

Uh oh!

youknowone Jul 19, 2026

Choose a reason for hiding this comment

Uh oh!

devyubin Jul 20, 2026 • 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!

youknowone Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

devyubin commented Jul 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 18, 2026 •

edited

Loading

github-actions Bot commented Jul 18, 2026 •

edited

Loading

devyubin Jul 20, 2026 •

edited

Loading