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 <jun66j5_at_gmail.com> wrote:
>>
>>> (Posting to dev list, again...)
>>>
>>> On Thu, Nov 14, 2019 at 1:47 AM Julian Foad <julianfoad_at_apache.org>
>>> 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
Received on 2019-11-15 08:37:47 CET