ethe / pygraphy

A modern Pythonic GraphQL implementation.
MIT License
92 stars 13 forks source link

Support for self-referencing input types #29

Closed lostb1t closed 4 years ago

lostb1t commented 4 years ago

Currently this results in a recursive error

Example code (python 3.7+):

from __future__ import annotations
import pygraphy

class WhereInput(pygraphy.Input):
    _and: WhereInput = None

Error output:

  File "/usr/local/lib/python3.7/site-packages/pygraphy/types/input.py", line 25, in validate
    field.ftype.validate()
  File "/usr/local/lib/python3.7/site-packages/pygraphy/types/input.py", line 25, in validate
    field.ftype.validate()
  [Previous line repeated 960 more times]
  File "/usr/local/lib/python3.7/site-packages/pygraphy/types/input.py", line 17, in validate
    field.ftype, except_types=(ObjectType, UnionType)
  File "/usr/local/lib/python3.7/site-packages/pygraphy/types/field.py", line 62, in ftype
    return self.replace_forwarded_type(self._ftype)
  File "/usr/local/lib/python3.7/site-packages/pygraphy/types/field.py", line 72, in replace_forwarded_type
    return self.get_type(ptype)
  File "/usr/local/lib/python3.7/site-packages/pygraphy/types/field.py", line 83, in get_type
    actual_type = _eval_type(ref, base_globals, None)
  File "/usr/local/lib/python3.7/typing.py", line 263, in _eval_type
    return t._evaluate(globalns, localns)
  File "/usr/local/lib/python3.7/typing.py", line 469, in _evaluate
    is_argument=self.__forward_is_argument__)
  File "/usr/local/lib/python3.7/typing.py", line 139, in _type_check
    if isinstance(arg, (type, TypeVar, ForwardRef)):
RecursionError: maximum recursion depth exceeded in __instancecheck__
ethe commented 4 years ago

I think it is a bug, thank you, I will fix it.

ethe commented 4 years ago

The situation is more complex than I expected, the root cause is that Pygrahy check validations at the type definition time. However, Python load definitions sequential, it means WhereInput is not in the context when Pygraphy starts to check it.

I will think about how to fix it, maybe we can make validation lazy, after all types have been initialized.

lostb1t commented 4 years ago

yeah, resolving forward refs after class creation or make them lazy might work. Pydantic does something similar: https://pydantic-docs.helpmanual.io/usage/postponed_annotations/

Lazy loading should be cleaner though instead of having to call a resolve function manually.

ethe commented 4 years ago

I fixed it, but according to the behavior of Python, we have to take care of definition sequences, I wrote a test case to explain it: https://github.com/ethe/pygraphy/blob/85745100c7af4981305eae1b7abb2c0ba7e8b1c8/tests/test_recursive_def.py

ethe commented 4 years ago

Thank you, if you have any other suggestions and bug reporting, please let me know, I will be glad to improve Pygraphy.