Skip to content

Fix BitStream pos not reset to 0 when content replaced via dtype setter#366

Open
gaoflow wants to merge 2 commits into
scott-griffiths:mainfrom
gaoflow:fix-bitstream-pos-reset-on-dtype-setter
Open

Fix BitStream pos not reset to 0 when content replaced via dtype setter#366
gaoflow wants to merge 2 commits into
scott-griffiths:mainfrom
gaoflow:fix-bitstream-pos-reset-on-dtype-setter

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 25, 2026

Copy link
Copy Markdown

Summary

Fixes issue #266: when a dtype property setter (e.g. a.u8 = 14) replaces the internal content of a BitStream, the bit position pos was silently left unchanged instead of being reset to 0.

>>> a = BitStream('0x0120310230123', pos=23)
>>> a.u8 = 14
>>> a.pos  # was 23 — should be 0
23          # BUG: should be 0

All other mutating operations (prepend, __setitem__, __delitem__, insert) already reset pos to 0 after changing the content. This makes the dtype-setter path consistent with the rest of the API.

Fix

Added BitStream.__setattr__ that compares the identity of _bitstore before and after delegating to super().__setattr__. When a dtype setter replaces the bitstore, _pos is reset to 0. Removed the @pytest.mark.skip("Bug #266") marker from the pre-existing regression test, which now passes.

Test

tests/test_bitstream.py::TestAllAndAny::test_pos_reset_bug PASSED

769 tests pass, no regressions introduced.

This pull request was prepared with the assistance of AI, under my direction and review.

When a dtype property setter (e.g. a.u8 = 14) replaces the internal
bitstore of a BitStream, the bit position was silently left unchanged
(issue scott-griffiths#266). All other mutating operations (prepend, __setitem__,
__delitem__, insert) already reset pos to 0 after a length change.

Add BitStream.__setattr__ to detect when a dtype setter replaced
_bitstore (detected by id change) and reset _pos to 0 accordingly.
Remove the @pytest.mark.skip("Bug scott-griffiths#266") marker from the regression test.
@codacy-production

codacy-production Bot commented Jun 25, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 6 complexity · 0 duplication

Metric Results
Complexity 6
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@gaoflow

gaoflow commented Jun 25, 2026

Copy link
Copy Markdown
Author

I pushed a small follow-up for the Codacy documentation warning: the new BitStream.__setattr__ override now has a docstring explaining why it resets pos when content is replaced.

Verification run locally:

python -m pytest tests/test_bitstream.py::TestAllAndAny::test_pos_reset_bug -q
git diff --check
uv run --with pytest --with hypothesis --with pytest-benchmark python -m pytest -q --ignore=tests/test_fp8.py --ignore=tests/test_mxfp.py

The ignored FP8/MXFP files require gfloat; with the latest gfloat, two tests fail because its format_info_p3109() signature has changed, so I kept the broad verification to the non-gfloat suite (773 passed, 3 skipped).

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.

1 participant