Skip to content

Honor explicit start offsets in load - #117

Merged
dpranke merged 1 commit into
dpranke:mainfrom
Metis-dot:fix/load-explicit-start
Oct 1, 2026
Merged

dpranke merged 1 commit into
dpranke:mainfrom
Metis-dot:fix/load-explicit-start

Conversation

@Metis-dot

Copy link
Copy Markdown
Contributor

Summary

Honor the documented absolute start argument in load(): when it is explicitly supplied, seek to zero before reading the full content and delegate the decoded-content offset to parse().

Currently, load() reads from the stream's existing position and applies start to that shortened content. This can silently change numeric values, as well as reject a valid document after the stream has been read to EOF.

import io
import json5

fp = io.StringIO('123')
fp.seek(1)
json5.load(fp, start=0)  # before: 23; after: 123

fp = io.StringIO('x 123')
fp.seek(2)
json5.load(fp, start=2)  # before: 3; after: 123

The existing load() docstring explicitly distinguishes start=None from an explicit offset and requires a seekable stream for the latter.

The two-line guard preserves the default: start=None continues reading from the current stream position without calling seek(). Explicit start=0 also follows the documented seek requirement. Negative/out-of-range offset handling and consume_trailing behavior remain delegated to the existing parser.

Verification

  • Existing baseline: 75 tests and doctests passed
  • Added regression tests fail on unpatched main, including 123 != 23 and 123 != 3
  • Patched full suite: 79 tests passed, including CLI subprocess tests against the checkout; doctests passed
  • Regression coverage includes text/binary streams, Unicode prefixes, a nonzero initial position, EOF, and default compatibility with non-seekable streams
  • Ruff check and format, Pylint, mypy, and git diff --check passed
  • Independently verified the published PyPI 0.15.0 wheel and the patch against current main

AI disclosure: An OpenAI coding assistant investigated, authored, and tested this patch, with independent review by another assistant, under the account owner's authorization to contribute to open-source projects.

@dpranke

dpranke commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Ooh, good catch! I can't believe I didn't write tests for this when I added the feature :(. Nice tests, also :).

@dpranke
dpranke marked this pull request as ready for review October 1, 2026 19:38
@dpranke
dpranke merged commit 28c8d86 into dpranke:main Oct 1, 2026
7 checks passed
@dpranke

dpranke commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Thanks for the fix!

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