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
Allow collections.abc.Callable to be used as type in python 3.9 #2519
Changes from all commits
ba3f554
7573c09
8808da0
6b3562d
829d12c
173b418
5ce721b
117d1c2
d5398e0
069e5b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Allow `collections.abc.Callable` to be used as type in python 3.9. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,18 @@ | ||
import sys | ||
from typing import Callable | ||
|
||
import pytest | ||
|
||
from pydantic import BaseModel, ValidationError | ||
|
||
collection_callable_types = [Callable, Callable[[int], int]] | ||
if sys.version_info >= (3, 9): | ||
from collections.abc import Callable as CollectionsCallable | ||
|
||
@pytest.mark.parametrize('annotation', [Callable, Callable[[int], int]]) | ||
collection_callable_types += [CollectionsCallable, CollectionsCallable[[int], int]] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could do this with all other generic aliases (Iterable, Sequence and so on). |
||
|
||
|
||
@pytest.mark.parametrize('annotation', collection_callable_types) | ||
def test_callable(annotation): | ||
class Model(BaseModel): | ||
callback: annotation | ||
|
@@ -14,7 +21,7 @@ class Model(BaseModel): | |
assert callable(m.callback) | ||
|
||
|
||
@pytest.mark.parametrize('annotation', [Callable, Callable[[int], int]]) | ||
@pytest.mark.parametrize('annotation', collection_callable_types) | ||
def test_non_callable(annotation): | ||
class Model(BaseModel): | ||
callback: annotation | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From your benchmarks (and mine), the following seems to be faster:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my benchmarks:
I have absolutely no idea why
is_none_type3
is so much slower thanis_none_type1
, but it does seem to be.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelcolvin sorry, I'm not sure if we're on the same page here.
I would also like to use the tuple version (is_none_type1 in your comment),
But this
was the reason I didn't want to do it (not sure if you saw that part of my comment).
What I ended up doing now is to add another version switch (python < 3.8). Makes the code a little less readable, but I guess it's not the end of the world and is easy to remove when 3.7 goes out of scope. Should get us the best of both worlds (fastest in 3.8 and 3.9 but still works in 3.6/3.7).
Hope that's an ok solution.