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

Jump to bottom

Allows Optional lists with unique_items check #4568

Merged
merged 1 commit into from Oct 17, 2022
Merged

Allows Optional lists with unique_items check #4568

merged 1 commit into from Oct 17, 2022

Conversation

mfulgo
Copy link

@mfulgo mfulgo commented Sep 26, 2022 •
edited by samuelcolvin

When using unique_items with an Optional[List[T]] field, the field validator would raise the following error if the field was not provided:

'NoneType' object is not iterable (type=type_error)

Updating the validator to return None in these cases avoids the issue.

Fixes #3957, fix #4050, fix #4119

@mfulgo
Allows Optional lists with unique_items check
3ba804e 
When using `unique_items` with an `Optional[List[T]]` field, the field
validator would raise the following error if the field was not provided:
> `'NoneType' object is not iterable (type=type_error)`

Updating the validator to return `None` in these cases avoids the issue.

Fixes pydantic#3957, pydantic#4050, pydantic#4119
@hramezani
Copy link
Member

Thanks @mfulgo for this PR 👍

The fix LGTM

But, As it was a problem before and has not been introduced in 1.10.x I am not sure that we want to include it. IMHO, we can ignore this change as it's not critical and wait for the proper fix in V2.

@samuelcolvin What do you think?

@mfulgo
Copy link
Author

mfulgo commented Sep 26, 2022

I hear that, but I expect that folks will depend on 1.10.x for a while before being able to switch to V2. So bug fixes to this branch (and you could cherry pick this commit to 1.9.x) seem like a good thing to me.

In any case, thanks for y'alls efforts and consideration!

@raymondberg
Copy link

Thank you so much for this PR! We'd love to get this merged soon. Just adding some ❤️ and open source downstream customer 😄 .

@hramezani
Copy link
Member

Please review

samuelcolvin
samuelcolvin approved these changes Oct 11, 2022
View reviewed changes
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

I hear that, but I expect that folks will depend on 1.10.x for a while before being able to switch to V2.

reluctantly, i think I agree with you.

I'm happy with this, @PrettyWood and @hramezani please confirm you're happy with this (or just merge if you are).

@samuelcolvin samuelcolvin mentioned this pull request Oct 11, 2022
Closed
8 tasks
@hramezani
Copy link
Member

I hear that, but I expect that folks will depend on 1.10.x for a while before being able to switch to V2.

reluctantly, i think I agree with you.

I'm happy with this, @PrettyWood and @hramezani please confirm you're happy with this (or just merge if you are).

Fine for me

@samuelcolvin samuelcolvin merged commit b516de7 into pydantic:1.10.X-fixes Oct 17, 2022
@samuelcolvin
Copy link
Member

thanks so much @mfulgo.

@mfulgo mfulgo deleted the 1.10.X-fixes branch October 17, 2022 15:26
@samuelcolvin samuelcolvin mentioned this pull request Oct 17, 2022
Closed
@nuno-andre nuno-andre mentioned this pull request Oct 17, 2022
Closed
15 tasks
@WilliamDEdwards
Copy link

When is v1.10.3 expected to be released?

@samuelcolvin
Copy link
Member

I'll try work on next week, any help with #4552 would be VERY MUCH appreciated and would help get the release out asap.

@WilliamDEdwards
Copy link

I'll try work on next week, any help with #4552 would be VERY MUCH appreciated and would help get the release out asap.

I can't help with code, so: #4631

@WilliamDEdwards WilliamDEdwards mentioned this pull request Dec 13, 2022
Closed
16 tasks
@Kludex Kludex mentioned this pull request Apr 30, 2023
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samuelcolvin samuelcolvin samuelcolvin approved these changes

Assignees

@hramezani hramezani

@samuelcolvin samuelcolvin

@PrettyWood PrettyWood

Labels
ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mfulgo @hramezani @raymondberg @samuelcolvin @WilliamDEdwards @PrettyWood