mila-iqia / myia

Myia prototyping
MIT License
455 stars 46 forks source link

Keep type annotations in graph, if available. #370

Closed notoraptor closed 4 years ago

notoraptor commented 4 years ago

@breuleux @abergeron This is a PR to parse annotations (for variables, function arguments, function return) and keep it in new class member ANFNode.annotation.

What do you think?

notoraptor commented 4 years ago

@breuleux code updated!

NB: I notice that this function f will be parsed (with imports at top of file):

from numpy import ndarray

def f(x: ndarray):
    return x * x

But parsing this function f will generate an error from eval because it can not find numpy (local import):

def wrapper_function():
  from numpy import ndarray

  def f(x: ndarray):
      return x * x

It's as if f.__globals__ won't include local numpy import from container function wrapper_function.

breuleux commented 4 years ago

@notoraptor That's true, it doesn't include closure variables. It's possible to get closure variables, though. Search for ClosureNamespace in the code, that class builds an internal dictionary of closure variables. eval takes a second argument for the locals, so maybe it could be made to work this way.

abergeron commented 4 years ago

According to PEP563, the postponed evaluation explicitly doesn't support the wrapper_function() case. The python compiler doesn't always keep enough information around to resolve that case so I don't think we should attempt that.

Also postponed-by-default is not until python 3.10, but can be enabled with a __future__ import.

breuleux commented 4 years ago

@abergeron I see. Well, in that case, I suppose we don't need to support it.

notoraptor commented 4 years ago

@breuleux @abergeron using ClosureNamespace seems to fix the parsing. I added a test to check it.