Skip to content

Fix unpacking with dynamic offsets#57

Merged
MatrixEditor merged 1 commit into
MatrixEditor:fix/dynamic-offsetsfrom
tsheinen:fix-dynamic-offset-unpack
Feb 6, 2026
Merged

Fix unpacking with dynamic offsets#57
MatrixEditor merged 1 commit into
MatrixEditor:fix/dynamic-offsetsfrom
tsheinen:fix-dynamic-offset-unpack

Conversation

@tsheinen
Copy link
Copy Markdown
Contributor

@tsheinen tsheinen commented Feb 5, 2026

_ContextLambda is ExprMixin which overwrites __eq__. The __contains__ function for tuples delegates to __eq__ and this interaction means that _has_offset and _keep_pos will be unconditionally false and true when using a dynamic offset.

Pack doesn't seem to work either, but I wasn't able to puzzle out why. I added tests here to snapshot the current behavior.

`_ContextLambda` is `ExprMixin` which overwrites `__eq__`. The
`__contains__` function for tuples delegates to `__eq__` and this
interaction means that `_has_offset` and `_keep_pos` will be
unconditionally false and true when using a dynamic offset.
MatrixEditor added a commit that referenced this pull request Feb 6, 2026
---

Move all parsing and building related functions (pack*, unpack*, sizeof) into a separate file for easy import handling.

Refs: #57
@MatrixEditor MatrixEditor changed the base branch from master to fix/dynamic-offsets February 6, 2026 20:59
@MatrixEditor
Copy link
Copy Markdown
Owner

Hi @tsheinen,

thanks for pointing out the issue with arbitrary offsets. The changes you proposed will fix unpacking - the same issue of context-lambda comparisons repeats across the Field implementation (--> added to TODO). The new "base" branch for this pull-request implements the necessary changes to pack objects with an arbitrary offset (internal logic was flawed) (ref: #58) .

I would like to merge this request if there are no other questions regarding this topic.

@tsheinen
Copy link
Copy Markdown
Contributor Author

tsheinen commented Feb 6, 2026

sounds good to me, thanks!

@MatrixEditor MatrixEditor merged commit fece147 into MatrixEditor:fix/dynamic-offsets Feb 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants