QuantStack / py2vega

BSD 3-Clause "New" or "Revised" License
9 stars 4 forks source link

Add Python 3.4 and 2.7 support #23

Closed martinRenou closed 5 years ago

martinRenou commented 5 years ago

@serge-sans-paille it seems that I get False and True as gast.Name and not gast.NameConstant in Python 2.7, because it triggers the NodeVisitor.visit_Name method. Should I be doing something more than simply replacing ast by gast in my code?

serge-sans-paille commented 5 years ago

@martinRenou : it's the py3.4 and before behavior, so there is no way around: if you want to support these versions, you need to handle the case where True and False are not reserved names. And thanks to gast you can handle this in both codes. Considering the change, basically read gast README, each difference between gast and python ast (for each version) is emphasized there

serge-sans-paille commented 5 years ago

btw a typical pattern is import gast as ast

martinRenou commented 5 years ago

@serge-sans-paille I am not sure I get it... According to the gast README:

In Python3, None, True and False are parsed as NamedConstant while they are parsed as regular Name in Python2. gast uses the same convention

I understand that gast handle it automatically and turn False, True and None into NamedConstant, without having to make a workaround. Do I understand this wrong?

martinRenou commented 5 years ago

Ok yes, I see I understand it wrong now. This line is actually saying the behavior does not change for both Python2 and Python3, is that it?

Then I'm not sure we need gast in py2vega, because all the other use cases handled by gast will have no effect on py2vega. py2vega does not allow functiondef, classdef, with, raise, tryexcept or comprehension. Note: We might add list comprehension support at some point by unrolling it in the vega-expression. But for the other nodes I don't think we'll ever support them.

martinRenou commented 5 years ago

Maybe Python 3.8 support in gast will have an effect on py2vega?

serge-sans-paille commented 5 years ago

Yes, bacause of the ast node ast.Constant that makes ast.Num obsoletes

martinRenou commented 5 years ago

Ok :) I'll add the gast dependency for the Python 3.8 support then. Did you not consider having the same behavior for Python 2 and Python 3 concerning the NamedConstant? Is it not possible?

serge-sans-paille commented 5 years ago

It's not trivial:

from mymodule import *
print(True)

Is a valid Python2 input, and you cannot state if True is a NamedConstant or a Name, depending on the (unknown) content of mymodule