On 04.01.2018 02:48, Troy Curtis Jr wrote: > > > On Tue, Jan 2, 2018 at 3:12 AM Branko Čibej <br...@apache.org > <mailto:br...@apache.org>> wrote: > > On 31.12.2017 03:05, Troy Curtis Jr wrote: > > > > This all makes sense and seems nice on the surface, but I'm not > > sure we > > can just change the behaviour of the bindings from old-style to > > new-style classes in a Python 2.x build. There are enough subtle > > differences in behaviour between the two that existing > scripts could > > break after an upgrade of the bindings. > > > > Python 3.x has only new-style (or rather, even-newer-style) > > classes and > > there's no backward-compatibility consideration, since our > bindings > > currently don't work with Python3. > > > > > > That is a reasonable concern. I definitely preferred the cleaner > > single implementation, but honestly the code necessary to > continue to > > use classic classes in python 2 is not large. I've attached a > working > > patch for reference/discussion. It is a bit more code and some > > conditional definitions, but perhaps it is the more preferred course > > to take? > > > > [[[ > > On branch swig-py3: Go back to using classic classes for Python > 2 swig > > bindings. > > > > Add some additional clarifying comments for the reasons behind > overriding > > __getattr__ and __getattribute__. > > > > * build/ac-macros/swig.m4 > > (SVN_FIND_SWIG): Add the '-classic' flag to swig when python 2 is > > detected. > > > > * subversion/bindings/swig/include/proxy.py > > (_retrieve_swig_value): Factor out metadata retrieval from > > __getattribute__ to a new function. > > (__getattribute__): Only define __getattribute__ for new > style classes. > > (__getattr__): Add back implementation for classic classes. > > ]]] > > > > Troy > > [...] > > > +· # SWIG classes generated with -classic do not define this > variable, > > +· # so set it to 0 when it doesn't exist > > +· try: _newclass > > +· except NameError: _newclass = 0 > > I prefer to break the try/except blocks onto separate lines, and > to use > None for the tristate idiom value: > > try: > _newclass > except NameError: > _newclass = None > > > Using None here is certainly more Pythonic, but in this case I was > trying to match up with what swig generates: > > try: > _object = object > _newclass = 1 > except __builtin__.Exception: > class _object: > pass > _newclass = 0 > > In this case we only need the _newclass variable defined, and not the > "empty" class definition. In all the conditional cases which use that > value, either way should work, but I think it is likely better to > stick with 0 for consistency in this case. > > However, I can understand the formatting request. > > Other than that, is the consensus that we should continue with classic > classes in Python 2 with the conditional logic, or use a common > implementation for the python2/python3 code like is currently in the > swig-py3 branch?
The differences between old-style and new-style classes is tricky. Offhand I can think of differences in invocation of base class constructors, for example. I'd say leave old style classes for Python 2 precisely because the change would not be 100% backwards compatible. -- Brane
