It looks like making this isinstance will allow type sub-classes for all the different constant node types. I guess this only affects 'ast' module users that are using the old class names? Just wondering if it is safe to allow any subtype in there. The constants need to get marshaled don't they?

I don't think that serialization has to be considered in isinstance().

It looks like making this isinstance will allow type sub-classes for all the different constant node types.

I don't understand. The second argument is _const_types[cls], not _const_types. ast.Str only check for str and str subclasses.

I mean the change affects all the constant node types (Str, Num, Bytes, etc). I was wondering aloud if that was okay. I looked at the new ast.py code again today and tested older versions of Python. I think this change is fine. It only affects code that uses the backwards compatibility node types. The _ABC.__instancecheck__ is not enough to make perfect backwards compatibility. Old versions of Python didn't do any type checking for ast node values so you could do something like:

    x = ast.Str({})
    assert isinstance(x, ast.Str)

That will fail with the new ast.py module, even after this PR. Maybe the following would be better. Make Constant "remember" what kind of constant node it is when created. Then have __instancecheck__ use that as the first test.

--- a/Lib/ast.py
+++ b/Lib/ast.py
@@ -338,6 +338,9 @@ Constant.s = property(_getter, _setter)
 class _ABC(type):
 
     def __instancecheck__(cls, inst):
+        const_type = getattr(inst, '_const_type', None)
+        if const_type and const_type is cls:
+            return True
         if not isinstance(inst, Constant):
             return False
         if cls in _const_types:
@@ -351,7 +354,9 @@ class _ABC(type):
 
 def _new(cls, *args, **kwargs):
     if cls in _const_types:
-        return Constant(*args, **kwargs)
+        n = Constant(*args, **kwargs)
+        n._const_type = cls
+        return n
     return Constant.__new__(cls, *args, **kwargs)
 
 class Num(Constant, metaclass=_ABC):

This can be done simpler if Str.__new__ return Str instead of Constant. But I think that since old classes will be removed eventually, it is better to return the pure Constant.

I don't think we should support ast.Str({}).

Can you please add this test?
self.assertFalse(isinstance(ast.Constant(S('42')), ast.Num))

(maybe instanciate the ast object only once)

I think it looks clearer if the ast object is instantiated at the same line. In most tests different ast objects are tested in sequential lines.