Skip to content

Narrow encode_into offset stub type to int - #1163

Merged
Siyet merged 1 commit into
msgspec:mainfrom
dylanpulver:fix-encode-into-offset-stub
Sep 14, 2026
Merged

Siyet merged 1 commit into
msgspec:mainfrom
dylanpulver:fix-encode-into-offset-stub

Conversation

@dylanpulver

Copy link
Copy Markdown
Contributor

Widening the runtime to accept None was the alternative. I rejected it: the docstring specifies offset : int, optional and reserves -1 as the sentinel for writing at the end of the buffer, which leaves None with no meaning to carry. Narrowing the stub makes the declared type match both the C code and the documented contract.

The int | None traces back to the original Optional[int] annotation and survived the 3.10+ syntax migration in #1041. Nothing in the tree passes None.

Residual limitation: this edits the same two stub lines as open PR #1160, which reshapes buffer and the return type while keeping offset: int | None. Whichever lands second needs a one-line rebase. The two changes are otherwise independent, since #1160 keeps reading offset via PyLong_AsSsize_t.

Measurement, CPython 3.14:

  • just test-typing clean under mypy 2.3.0, pyright 1.1.411, pyrefly 1.2.0.
  • Reverting the stub hunk alone makes all three checkers fail on both new call sites in basic_typing_examples.py, so the type-level test is not vacuous.
  • pytest tests/unit: 6119 passed, 404 skipped, identical to the pre-change baseline. The two runtime assertions are pins on current behavior rather than new coverage.
  • ruff format collapses the signature onto one line once | None is gone; that reformat is included.

No changelog entry, following merged stub PRs #1055, #1057, #1062, #1065, #1074 and #1093, which leave it to the aggregated "type stub improvements" line at release.

The type stubs for `msgspec.json.Encoder.encode_into` and
`msgspec.msgpack.Encoder.encode_into` declared `offset: int | None = 0`,
but the C implementation passes the argument to `PyLong_AsSsize_t` and
raises `TypeError: an integer is required` for `None`. The docstring
also documents `offset : int, optional`, with `-1` (not `None`) as the
sentinel for writing at the end of the buffer.

Narrow the stubs to `int` and add type-level and runtime regression
tests.

@Siyet Siyet left a comment

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.

Thx for the fix and for adding regression tests for both encoders!

@Siyet
Siyet added this pull request to the merge queue Sep 14, 2026
Merged via the queue into msgspec:main with commit 69bc5b7 Sep 14, 2026
27 of 28 checks passed
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.

2 participants