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

Bug fix for forward refs in generics #6157

Merged
merged 5 commits into from Jun 23, 2023

Conversation

mark-todd
Copy link
Contributor

@mark-todd mark-todd commented Jun 15, 2023

Change Summary

Converts string to Forward Refs to allow for generic case
Also to fix discriminated unions allows Literals to pass through replace types unchanged.

Related issue number

fix #6130

Selected Reviewer: @dmontagu

@mark-todd
Copy link
Contributor Author

please review

@dmontagu
Copy link
Contributor

I see this is breaking one of the discriminated union tests, do you have an idea of how to resolve that? If we can get that fixed I'd be happy to merge this.

@mark-todd
Copy link
Contributor Author

Hi David - so I've been trying to figure this out, this much I know:

The test used to fail for me in python3.10, then I added the check for is Literal (it was failing because the string in the literal was being converted to a ForwardRef)

I fixed it, but then when I uploaded I found it was producing the same bug in python3.9 and below, but not 3.10 or 3.11. I don't understand this because it should use the same function. It must be something to do with the conditional imports based on version that are changing the behaviour. I'll look into it some more and see what I can find but any thoughts on it would be appreciated!

@mark-todd
Copy link
Contributor Author

I've had a follow up thought on this - 3.9 or less does not have the list dict features, so it's possible there is a different part of the code that executes instead of replace args but that has a similar role.

@mark-todd
Copy link
Contributor Author

Getting there! Now works for 3.8 and up - seems to be still something wrong on 3.7

@mark-todd
Copy link
Contributor Author

@dmontagu All tests fixed - should be good to merge

@dmontagu dmontagu merged commit 51b4751 into pydantic:1.10.X-fixes Jun 23, 2023
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants