Split csfield into csfield, csfield_noinit, csfield_const and csfield_default#41
Open
timrid wants to merge 2 commits into
Open
Split csfield into csfield, csfield_noinit, csfield_const and csfield_default#41timrid wants to merge 2 commits into
timrid wants to merge 2 commits into
Conversation
…`csfield_const` and `csfield_default`.
There was a problem hiding this comment.
Pull request overview
This PR makes DataclassStruct dataclass field intent explicit by splitting the former single csfield helper into four dedicated field specifiers (csfield, csfield_noinit, csfield_const, csfield_default), improving runtime behavior clarity and enabling more accurate static type checking (notably with Pyright). It also updates tests, documentation, and stubs to reflect the new APIs and correct build-from-None semantics.
Changes:
- Introduces new field-specifier helpers and applies
@dataclass_transformto improve type-checker understanding of generated dataclass constructors. - Expands/adjusts tests (including a new Pyright-focused test module) to validate constructor parameter behavior and validation errors.
- Updates README/CHANGELOG and improves stubs (notably
Checksum) to reflectbuild(None)capability.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
construct_typed/dataclass_struct.py |
Splits field creation into four helpers + adds dataclass_transform field specifiers. |
construct_typed/__init__.py |
Exports the new field helpers from the package API surface. |
construct-stubs/core.pyi |
Adjusts Checksum build typing to allow None builds. |
tests/test_typed.py |
Updates/extends runtime tests for the new field behaviors and validation. |
tests/test_typed_pyright.py |
Adds Pyright regression tests for missing/extraneous dataclass constructor parameters. |
README.md |
Updates examples and adds new sections documenting the new helpers. |
pyproject.toml |
Excludes the Pyright-specific test from mypy to avoid intentional error noise. |
CHANGELOG.md |
Documents breaking changes and new helper APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -101,15 +101,15 @@ A short example: | |||
| import dataclasses | |||
| import typing as t | |||
| from construct import Array, Byte, Const, Int8ub, this | |||
|
|
||
| ```python | ||
| import dataclasses | ||
| from construct import Const, Int8ub, Bytes |
Comment on lines
+4
to
+5
| ** Breaking changes: ** | ||
| - `csfield` should only used for constructs that cannot build from `None`. Every other construct should use the new `csfield_noinit`, `csfield_const` or `csfield_default`. |
| - Removed `version.py`, use `importlib.metadata` instead to get the version number. | ||
| - Use `uv` as a project management tool and `poe` as a task runner. | ||
|
|
||
| ** New features: ** |
| - Removed wildcard imports, so it may be necessary to change some import statements in your code when you use non-public APIs. No newline at end of file | ||
| - Optimize type stubs for `Checksum`, to represent that it can build from `None`. | ||
|
|
||
| ** Organizational changes: ** |
| # Construct allows to put non-default values after Default. Dataclass and Pyright don't like that too much. It is necessary to | ||
| # specify the field `kw_only` and pass it "by keyword". | ||
| normal_int: int = csfield(cs.Int8ub, kw_only=True) | ||
| const_int2: int | None = csfield_const(cs.Int8ub, 5) |
Comment on lines
+133
to
+137
| return dataclasses.field( | ||
| default=const, | ||
| init=False, | ||
| metadata={"subcon": subcon}, | ||
| ) |
Comment on lines
+176
to
+180
| return dataclasses.field( | ||
| default=default, | ||
| init=True, | ||
| metadata={"subcon": subcon}, | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Previously, there was only a single
csfieldmethod to define dataclass fields forDataclassStruct. This led to ambiguity and potentially incorrect behavior:Computed,Rebuild,Tell,Const) had to be declared viacsfield, even though they can't provide a meaningful value when the instance is created — this resulted in unnecessary or incorrectly typed constructor parameters.Solution
csfieldhas been split into four specialized methods that make the intended behavior explicit:csfield– still used for constructs that cannot build fromNone(the default case, generates a required constructor parameter).csfield_noinit– for fields excluded from the constructor (e.g.Computed,Rebuild,Tell); the value is automatically set toNoneon construction, so the field type must beT | None.csfield_const– automatically wraps the value incs.Constand sets it directly, without a constructor parameter:csfield_default– wraps the value incs.Defaultand generates a constructor parameter with a default value (all following fields must bekw_only):Additional changes
Checksumwere optimized to correctly reflect whether the respective construct can build fromNone.Breaking Changes
csfieldshould now only be used for constructs that cannot build fromNone. For every other case, usecsfield_noinit,csfield_constorcsfield_default.(This pull request replaces #38)