Merged
Conversation
Cattrs works by passing the value it is structuring into the default
constructor of the target type, so it supports Python's coercions.
In this case, consider the previous version of TextData:
class TextData:
data: str
type: "text" | "file"
When a test suite containing {'type': 'text', 'data': 500} is
structured, cattrs will attempt to structute `data` by doing `str(500)`.
This is legal, so cattrs happily accepts it.
The new definition, however, is as follows:
class TextData:
content: ContentPath | str
path: str | None = None
When the decorator runs and returns the value from data as an int,
cattrs will take over, trying to structure it into `ContentPath | str`.
TESTed has a custom union structuring function to allow structuring
into any union. One thing it does not do, however, is the data coercion.
The union structuring requires exact types, which is why the code failed
and the old test suite no longer works.
To fix this, we now explicitly coerce the legacy data to a string inside
the fallback converter before it reaches the strict union hook.
chvp
approved these changes
Feb 28, 2026
Member
Author
|
Merging into a "files" branch so we can have all file-related stuff together before merging to master. |
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.
intas the input forstdin, but it did work, so we are stuck with it.For those interested, this is the actual cause, from the commit message:
Cattrs works by passing the value it is structuring into the default constructor of the target type, so it supports Python's coercions. In this specific case, consider the previous version of
TextData:When a test suite containing
{'type': 'text', 'data': 500}is structured, cattrs will attempt to structuredataby doingstr(500). This is legal, so cattrs happily accepts it.The new definition, however, is as follows:
When the decorator runs and returns the value from data as an
int, cattrs will take over, trying to structure it intoContentPath | str. TESTed has a custom union structuring function to allow structuring into any union. However, it does no data coercion: it requires exact types, which is why the code failed and the old test suite no longer works.