sumtype icon indicating copy to clipboard operation
sumtype copied to clipboard

Typechecking fails for recursive type annotations

Open Bobbias opened this issue 1 year ago • 2 comments

Consider the following:

from __future__ import annotations
from sumtype import sumtype

class Test(sumtype):
    def A(inner: Test): ...

This results in the following NameError exception being raised:

Traceback (most recent call last):
  File "scratch.py", line 4, in <module>
    class Test(sumtype):
    ^^^^^^^^^^^^^^^^^^^^
  File "...\Lib\site-packages\sumtype\sumtype_meta.py", line 65, in __new__
    hints = typing.get_type_hints(constructor_stub)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\Lib\typing.py", line 2315, in get_type_hints
    hints[name] = _eval_type(value, globalns, localns)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\Lib\typing.py", line 365, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\Lib\typing.py", line 842, in _evaluate
    eval(self.__forward_code__, globalns, localns),
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<string>", line 1, in <module>
NameError: name 'Test' is not defined

The future import is supposed to help avoid this problem by delaying the evaluation of all type annotations, but since you are explicitly calling typing.get_type_hints(), I believe you are effectively breaking that functionality. Further, using @typeguard_ignore does not fix this issue either.

This recursive definition is a common way to encode things like parse trees (which is exactly my current use case).

Bobbias avatar Sep 02 '22 00:09 Bobbias

first of all, wow, hi! always nice to see someone use this old thing. i'm gonna be honest, i haven't touched this code or done any serious python in a while, and i'm not sure if i'll have the time to fix this anytime soon. i'll try, but i can't promise anything, so if this is a blocker, you should probably fork or look for another library. sorry!

Fixing it

yeah, recursive definitions are always tricky, i didn't need them for the thing i wrote this library for, so i never ran into this 😅 now, i believe the only thing we need the types for is to splice them into the __annotations__ of the generated constructors so that @typechecked can pick them up. and afaict, typeguard itself is now able to deal with forward references, so i think we could just copy over the raw annotations (w/o calling get_type_hints) and let TG deal with it. what might be trickier is that i'm guessing TG expects typechecked to be called within YOUR module, not within sumtype's code, because the only way to automagically resolve typehints is to walk the callstack and poke around in the caller's module namespace (that's what typing does iirc). so some extra plumbing might be needed to give it access to the correct namespace... sounds doable but TG might not expose something like that in their API, i'd need to look into it.

Possible workarounds

you could of course do something like

_Tree = Any
class Tree:
  def Node(left: _Tree, right: _Tree): ...
  def Leaf(label: string): ...

but really that's just disabling the typecheck, so it's not great.

a slightly uglier (unless you like LISP :P) but typesafe solution would be to define your type "the namedtuple way":

_Tree = 'Tree'
Tree = sumtype('Tree', [
    ('Node', [('left', _Tree), ('right', _Tree)]),
    ('Leaf', [('label', string)]),
])

(from the docstring here)

iirc only the "class-based way" calls get_type_hints, so this should bypass the issue. (i remember there's some validation in there that might complain about receiving a string instead of a type... can't test it rn though)

lastly, if THAT fails, you could define it as untyped and try wrapping the constructors in @typechecked yourself. something like

Tree.Node = typechecked(Tree.Node)

but you'd need to get the annotations in there somehow.


but yeah, again, i'm not sure if i'll have time/space to fix this -- i've been away from python stuff for quite some time, and i'm pretty busy ATM. if you've got time to dig into this, i'd be more than happy to merge a PR or even give you commit/PyPI access -- this library has been gathering dust for quite some time, it could use some love 😄

lubieowoce avatar Sep 05 '22 00:09 lubieowoce

Well for the time being I did the really dumb thing and just replaced all recursive references with typing.Any to avoid being blocked on this. I'm happy to see a relatively fast reply though, I wasn't sure if I was even going to get a reply 😄 I found a few other sumtype libraries out there, but this one seemed the easiest to work with.

I only took a quick look into the insides of the library, and pretty quickly decided I didn't want to sidetrack my current project trying to work out a solution (this library was actually what introduced me to typeguard. I've used mypy before for typechecking, but that's about it), but depending on how things go, I might work on it later. It would probably be a good thing to get properly acquainted with typeguard.

I do like Lisp, but I also prefer to keep my Lisp with my Lisp, and not inside my python 😄 Replacing the recursive references with Any was by far the easiest refactor I could do to work around things. Plus, since not all my sumtypes actually have recursive references, switching notation would result in either a mishmash of both styles, or end up causing a lot more work to switch every sumtype over to that syntax, so I took the lazy option.

Bobbias avatar Sep 07 '22 17:09 Bobbias