On 2019/11/15 16:37, Branko Čibej wrote:
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.
Moreover, swig-py bindings for Python 2 always use classic style for it,
if we can decide not to support SWIG 4+ for Python 2. In this case,
the distinction is needed only for Python 2/3 dual support in
applications, but it is equivalent to Python 2/3 distinction in such
case.
We should add a note
about that to the 1.14 release notes, but otherwise, I'm all for
committing this patch.
I think it is also need some description on somewhere about other
difference between py2/py3 bindings, mapping from/to char * type in
C structures (and somethings else if they exist), but it is not a
reason to block to commit the patch.
Cheers,
--
Yasuhito FUTATSUKI <[email protected]>/<[email protected]>
