Anthony Sottile <asott...@umich.edu> added the comment:

The simplest case is just the addition of an `isinstance` check:

https://github.com/asottile/dead/blob/85f5edbb84b5e118beab4be3346a630e41418a02/dead.py#L165-L170

class V(ast.NodeVisitor):
    def visit_Str(self, node):
        ...

    def visit_Constant(self, node):
        if isinstance(node.value, str):
            self.visit_Str(node)
        else:
            self.generic_visit(node)


But I have another case where there's much more complex (sorry this one's 
private, I'll give the pseudo-code of it though which ends up in a whole mess 
of slow `isinstance(...)` calls


class V(ast.NodeVisitor):
    def visit_Str(self, node):
        ...

    def visit_Bytes(self, node):
        ...

    def visit_Num(self, node):
        ...

    def visit_Constant(self, node):
        if isinstance(node.value, str):
             return self.visit_Str(node)
        # Annoying special case, bools are ints, before this wouldn't call
        # `visit_Num` because there was a separate `visit_NameConstant` for 
`True` / `False`
        elif isinstance(node.value, int) and not isinstance(node.value, bool):
            return self.visit_Num(node)
        elif isinstance(node.value, bytes):
            return self.visit_Bytes(node)
        else:
            return self.generic_visit(node)


Whereas the opposite case (where handling all constants the same) is much much 
easier to simplify the code and not need the `Constant` mess (if required)

class V(ast.NodeVisitor):
    def _visit_constant(self, node):
        # generic handling of constant _if you want it_

    visit_Str = visit_Num = visit_Bytes = visit_NameConstant = visit_Ellipsis = 
_visit_constant


Maybe I haven't been in the community long enough but this is the first 
*removal* of the ast I've seen, and it makes all my uses of these functions 
objectively worse, and none of the cases get simpler (whereas there's an 
existing easy way to simplify constants already, without breaking the ast)

github search isn't a great measure of this but it's currently showing 10k+ 
usages of `visit_{Str,Num,Bytes}` that would presumably be broken by this 
change: https://github.com/search?q=visit_Num+visit_Str+visit_Bytes&type=Code 
-- backward compatibility is very valuable, I would hope it isn't squandered 
for an internal cleanup

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue36917>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to