On Tue, Aug 17, 2010 at 5:12 PM, Craig Citro <[email protected]> wrote: > Hi Stefan, > >> Ah, got it. This is just what Carl wrote in the initial bug report. The new >> setup fails to provide a __getattr__ method and implements a >> __getattribute__ method instead. But the problem is not only that the >> introspection says it's not there, the problem is that the method really is >> not there. So we break user code that tries to use __getattr__ as an >> unbound method *and* the code generated by Cython for setting and >> retrieving docstrings of this method, thus breaking the module >> initialisation. So it's a double regression that is clearly user visible. >> >> Sounds like a release blocker to me. >> > > Robert and I were chatting about this earlier -- did the changes you > pushed completely fix this problem, or just cover it up? We couldn't > come up with a case that was still a problem off the cuff, but the > commit message said something like "for now," which made us think that > there might be more to the story. ;) > > -cc
Well, Stefan's patch essentially reverts the part of my patch dealing with __getattr__. So this definitely fixes any __getattr__-related regressions, at the cost of not fixing the particular performance bug that was the reason for my patch (the one that William was so unhappy about on sage-devel). I have an idea for another patch that might make everyone happy. After calling PyType_Ready, copy __getattribute__ (the wrapper object provided by Python) to __getattr__; then the type has a __getattr__, so Stefan's code should work, but it's a Python wrapper object, which will hopefully avoid the performance problem on subclasses. (I'm not sure what this would do with respect to the docstring issue Stefan mentioned.) I could try this out this weekend, but if somebody else wants to give it a try sooner that would be great. Carl _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
