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

Fix List and Dict valid_data #1052

Merged
merged 1 commit into from Dec 26, 2018
Merged

Fix List and Dict valid_data #1052

merged 1 commit into from Dec 26, 2018

Conversation

lafrech
Copy link
Member

@lafrech lafrech commented Nov 15, 2018

I've been trying to fix a few issues about valid_data in List and Dict.

Fixes #766.

@lafrech lafrech added the bug label Nov 15, 2018
@lafrech lafrech added this to the 3.0 milestone Nov 15, 2018
@lafrech
Copy link
Member Author

lafrech commented Nov 15, 2018

A few words of explanation.

In List field, I applied the changes described in #766.

Since it is used in a Dict test, I ended up modfying both fields in the same PR.

In Dict field, I passed the valid data as valid_data kwarg rather than data (bugfix), and I modified the logic to ensure valid data contains items for which the key deserializes correctly (even if value deserializes correctly, it is worthless if the key is incorrect) and the value deserializes at least partially (valid_data is not None). I think it makes for a more useful valid data structure.

In the code before this change, the keys array is initialized with serialized keys and then each key is replaced with its deserialized version, but in case of ValidationError, the serialized key remains in the result dict.

One thing worth mentioning, and I noted it in a comment: [|de]serializetion outputs a simple dict whatever the input dict type. We could allow specifying the dict type, or even use a trick to return the same type (I'm thinking of using copy and clear, but that sounds terribly hackish).

@lafrech lafrech changed the title WIP - Fix List and Dict valid_data Fix List and Dict valid_data Nov 30, 2018
@lafrech
Copy link
Member Author

lafrech commented Nov 30, 2018

Just reviewed and removed the WIP in title as I think this is good to go.

@sloria sloria merged commit 46c11a4 into dev Dec 26, 2018
@sloria sloria deleted the fix_list_dict_valid_data branch December 26, 2018 23:32
@sloria sloria mentioned this pull request Jan 15, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List field does does not put anything in valid_data if an item fails validation
2 participants