On 15.11.2019 08:10, Yasuhito FUTATSUKI wrote: > On 2019/11/15 14:04, Nathan Hartman wrote: >> On Thu, Nov 14, 2019 at 8:18 AM Jun Omae <[email protected]> wrote: >> >>> (Posting to dev list, again...) >>> >>> On Thu, Nov 14, 2019 at 1:47 AM Julian Foad <[email protected]> >>> wrote: >>>> Perhaps someone could commit this if there are no other concerns with >>>> it, and adjust INSTALL at the same time? >>> >>> Updated proposed patch, including a change of >>> subversion/bindings/swig/INSTALL. >>> >> >> Hi all, >> >> Jun, thank you for this patch. >> >> I would like to get this committed quickly if there are no >> objections, but >> this is not my area of expertise. >> >> Brane, do I understand correctly that you are satisfied with the >> results of >> your review/test? > > I also tested make check-swig-py on FreeBSD, with Python 2.7/3.7 and > SWIG 2.0.0/2.0.12/3.0.9/3.0.10/3.0.12/4.0.1 combination. Of course, > combination of Python 2.7 + SWIG 4.0.1 and combination of > Python 3.7 + SWIG < 3.0.10 are blocked by .check_swig_py target :) > Other combinations are passed the test. > > I considered if we can move classic style versus new style class > conditional > from run time to SWIG code generation time only, but it is not just > problem in this patch only, but also in current code. > >> In particular, the main question I have is with regards to the -modern >> option being added for SWIG 3.x .. <4 with Py3. If I understand >> correctly, >> this changes the way SWIG will generate accessors (properties vs >> getters/setters). Would this break any downstream code? (And if so, >> is that >> acceptable?) > > There is no Python 3 application depending on it, because we have not yet > released SWIG Python bindings that supports Python 3. > >> Also I don't fully understand the last part of the patch: is it creating >> replacements for the aforementioned getters/setters? > > No, they are only helpers to create them to absorb difference how to > access > attributes between combination of Python/SWIG versions. I think this is > a private part how we implement proxy objects for C data structures, and > not for expose to be used from applications.
+1. While the modern/classic distinction is potentially visible from Python 2 code (especially when using metaclasses with swig-py bindings), I consider that very much a corner case. With the upcoming EOL of Python 2, this difference is quickly becoming irrelevant. We should add a note about that to the 1.14 release notes, but otherwise, I'm all for committing this patch. -- Brane
