Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve decoder deserialization #1599

Merged
merged 1 commit into from
Aug 6, 2024
Merged

Conversation

Narsil
Copy link
Collaborator

@Narsil Narsil commented Aug 6, 2024

Improves the actual error message when type is present, but fields are invalid.

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@Narsil Narsil force-pushed the improve_decoder_deserialization branch from eada410 to e28016c Compare August 6, 2024 11:37
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think we are losing a little bit with the custom, worth to declare outside error

@@ -98,7 +208,7 @@ mod tests {
match parse {
Err(err) => assert_eq!(
format!("{err}"),
"data did not match any variant of untagged enum DecoderWrapper"
"data did not match any variant of untagged enum DecoderUntagged"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could change the custom to properly output decoder wrapper

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the new message is better, since Untagged is more descriptive than wrapper (both mean nothing to regular users, but at least untagged points to the fact that the type is missing.

We can override the message as needed. Do you have a good suggestion for the message?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of the box, something like "The type field is not present and we could not load into any of the variants of DecoderUntagged"

@Narsil Narsil merged commit 7a30bca into main Aug 6, 2024
13 checks passed
@Narsil Narsil deleted the improve_decoder_deserialization branch August 6, 2024 14:42
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.

3 participants