Added partial support to Dynamic type #725
Conversation
There was a problem hiding this comment.
Hi @nikordaris , thanks for the pull request!
I've added a suggestion to make it better. Also if you could add some tests to assert the new behaviour that would be great. You should add them here: https://github.com/graphql-python/graphene/blob/12d4dab7742c9408777380c390152d7cef0f838d/graphene/types/tests/test_dynamic.py
| @@ -11,7 +12,7 @@ class Dynamic(MountedType): | |||
|
|
|||
| def __init__(self, type, with_schema=False, _creation_counter=None): | |||
| super(Dynamic, self).__init__(_creation_counter=_creation_counter) | |||
| assert inspect.isfunction(type) | |||
| assert inspect.isfunction(type) or isinstance(type, partial) | |||
There was a problem hiding this comment.
I think a better check would be to use callable then it would catch both cases.
There was a problem hiding this comment.
You can't use callable because class types are callable.
There was a problem hiding this comment.
I could do not inspect.isclass(type) and callable(type)
There was a problem hiding this comment.
Really though I was basing it off of
graphene/graphene/types/utils.py
Line 44 in 12d4dab
I wasn't sure if there were callable types we didn't want to support
There was a problem hiding this comment.
Ah yeah you're right. What you've done then looks good. Just needs some tests.
|
ok @jkimbo I've added a test for partial. let me know if there is anything else i should change |
|
It goods look to merge for me @jkimbo, feel free to merge once you review it :) |
|
@syrusakbary I can't merge, I need write access to do that. |
|
This has been released in v2.1.1 |
No description provided.