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: avoid RecursionError when using some types like Enum or Literal with generic models #2438

Merged
merged 4 commits into from Mar 3, 2021

Conversation

PrettyWood
Copy link
Member

@PrettyWood PrettyWood commented Mar 1, 2021

Change Summary

Since an Enum is an Iterable, which would cause an infinite loop.

Related issue number

closes #2436
closes #2454

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link

codecov bot commented Mar 1, 2021

Codecov Report

Merging #2438 (f5c24cb) into master (a8d50ae) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2438   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines         5080      5100   +20     
  Branches      1041      1048    +7     
=========================================
+ Hits          5080      5100   +20     
Impacted Files Coverage Δ
pydantic/generics.py 100.00% <100.00%> (ø)
pydantic/json.py 100.00% <0.00%> (ø)
pydantic/main.py 100.00% <0.00%> (ø)
pydantic/types.py 100.00% <0.00%> (ø)
pydantic/fields.py 100.00% <0.00%> (ø)
pydantic/_hypothesis_plugin.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8d50ae...f5c24cb. Read the comment docs.

pydantic/generics.py Outdated Show resolved Hide resolved
@samuelcolvin samuelcolvin added this to the v1.8.1 milestone Mar 2, 2021
@samuelcolvin
Copy link
Member

LGTM, @PrettyWood & @daviskirk I'll merge this soon if you're happy with it.

@PrettyWood
Copy link
Member Author

PrettyWood commented Mar 2, 2021

Thanks @samuelcolvin ! 🙏

I just updated the change description since it also solves #2454

@PrettyWood PrettyWood changed the title fix: support properly Enum when combined with generic models fix: avoid RecursionError when using some types with generic models Mar 2, 2021
@samuelcolvin
Copy link
Member

We should add a test for a literal too, just so we don't forget.

@PrettyWood PrettyWood changed the title fix: avoid RecursionError when using some types with generic models fix: avoid RecursionError when using some types like Enum or Literal with generic models Mar 2, 2021
@daviskirk
Copy link
Contributor

daviskirk commented Mar 2, 2021

Looks great to me thanks for fixing my oversights, just tried it out with the literal, works fine as well!

@skip_36
def test_generic_literal():
    T = TypeVar('T')

    class SomeGenericModel(GenericModel, Generic[T]):
        some_field: T

    class MyModel(BaseModel):
        my_gen: SomeGenericModel[Literal["foo"]]

    m = MyModel.parse_obj({'my_gen': {'some_field': "foo"}})
    assert m.my_gen.some_field == "foo"

    with pytest.raises(ValidationError, match=".*permitted: 'foo'.*"):
        MyModel.parse_obj({'my_gen': {'some_field': "bar"}})

edit: @PrettyWood, you're really quick! Please Ignore my comment

@samuelcolvin samuelcolvin merged commit ab69114 into pydantic:master Mar 3, 2021
@samuelcolvin
Copy link
Member

🙏 thanks everyone.

@PrettyWood PrettyWood deleted the fix/generic-with-enum branch March 3, 2021 09: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
3 participants