On Tue, Aug 17, 2010 at 5:35 PM, Carl Witty <[email protected]> wrote: > 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.)
Copying it over isn't the right solution as __getattr_ is different than __getattribute__, and only the latter is created for a tp_getattr slot. I think that providing our own __getattr__ wrapper will not impact the performance of __getattribute__ (the important case), but this needs to be tested. - Robert _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
