classification
Title: Updated ASTVisitor Classes
Type: feature request Stage: test needed
Components: Extension Modules Versions: Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: jimjjewett, loupgaroublond (2)
Priority: normal Keywords: patch

Created on 2007-04-24 05:30 by loupgaroublond, last changed 2009-03-30 23:25 by ajaksu2.

Files
File name Uploaded Description Edit Remove
visitor.py loupgaroublond, 2007-04-24 05:30 updated ASTVisitor Class
visitor.py loupgaroublond, 2007-04-26 04:26 Revision 2
Messages (6)
msg52515 - (view) Author: Yaakov Nemoy (loupgaroublond) Date: 2007-04-24 05:30
Ok,

So this is really more of a complete rewrite than a bunch of patches, but it's just a picture of what IMHO is a saner way to do this module.  I suppose I could refactor alot of it to look more like the old module, but I'm still not sure what would be better, in terms of having seperate visitors to walkers.  Either way, the terminology in the original wasn't much clearer.

At bare minimum though, i got postorder implemented, so this code has got to be worth something valuable... I hope....

Feedback please.
msg52516 - (view) Author: Jim Jewett (jimjjewett) Date: 2007-04-24 13:53
I haven't looked deeply yet, but I know some feedback quickly is often better than a long wait.  I do have a few style suggestions.

(1)  Tests for None should generally use "is" rather than "==".

(2)  It helps to understand the code if it is more parallel.  Seeing the meth is/is not None cases reversed on the two orders confused me about how similar they were.

(3)  Inheriting from object (or setting __metaclass__ = type) is probably a good habit, since old-style classes will disappear in Py3K.  Normally, it doesn't matter, but debugging the difference is a pain.

To make the first two concrete, here is my rewording of ASTVisitor.dispatch:

From:

    def dispatch(self, node, *args):
        klass = node.__class__
        meth = self._get_method_for_className(klass)
        if self._order == self.PREORDER:
            if meth != None and meth(node, *args) != self.NO_CONTINUE:
                self._iterate_over_defaults(node, *args)
            elif meth == None:
                self._iterate_over_others(node, *args)
        elif self._order == self.POSTORDER:
            if meth == None:
                self._iterate_over_others(node, *args)
            if meth != None:
                self._iterate_over_defaults(node, *args)
                meth(node, *args)

To:

    def dispatch(self, node, *args):
        klass = node.__class__
        meth = self._get_method_for_className(klass)
        if meth is None:
            self._iterate_over_others(node, *args)
        else:
            if self._order == self.POSTORDER:
                self._iterate_over_defaults(node, *args)
                meth(node, *args)
            elif meth(node, *args) != self.NO_CONTINUE:
                self._iterate_over_defaults(node, *args)
            else:
                pass # pre-order, and the method said to stop

msg52517 - (view) Author: Yaakov Nemoy (loupgaroublond) Date: 2007-04-24 21:03
That looks like a good idea.  I just hammered the code out last night, and I'll try to include your changes tonight if I get a chance.  I'll take a look at #3, but I'm not quite familiar with the Py3k changes as I should be.
msg52518 - (view) Author: Jim Jewett (jimjjewett) Date: 2007-04-24 21:16
> I'll take a look at #3, but I'm not quite familiar with the Py3k changes 

Don't worry too much -- for the most part, Py3k is just a matter of cleanup.  For reasonable code, there usually isn't much difference between

    class Old: ...

and 

    class New(object): ...

But this means that when there is a difference, it is a pain to debug.  So just stick with the version that will continue to be available (inheriting from object), and backwards-compatibility will take care of itself.
msg52519 - (view) Author: Yaakov Nemoy (loupgaroublond) Date: 2007-04-26 04:26
Ok, I made your recommended changes, and added a scanty bit of documentation.

Just wondering, how long till such a change would be put upstream?  I'm going to use this in a project.
File Added: visitor.py
msg52520 - (view) Author: Jim Jewett (jimjjewett) Date: 2007-04-26 18:47
You are welcome to use it yourself, and ship it with your project.  

It almost certainly won't be added to the standard distribution until the next feature release.  PEP 361 http://www.python.org/dev/peps/pep-0361/ says that is targeted around April of 2008.  (Alphas will be available sooner, particularly for python 3.x)
History
Date User Action Args
2009-03-30 23:25:38ajaksu2setstage: test needed
type: feature request
versions: + Python 2.7, - Python 2.5
2007-04-24 05:30:12loupgaroublondcreate