Python wrapper in SWIG - Githubissues

apertium / lttoolbox

Finite state compiler, processor and helper tools used by apertium

http://wiki.apertium.org/wiki/Lttoolbox

GNU General Public License v2.0

18 stars 22 forks source link

Python wrapper in SWIG #58

Closed singh-lokendra closed 5 years ago

singh-lokendra commented 5 years ago

MVP for lt-proc -w
Changed AM_PATH_PYTHON to 3.4, as Python.h of python2.7 reports error with clang, due to ISO c++ 17

sushain97 commented 5 years ago

@Vaydheesh is this ready for @TinoDidriksen / @unhammer to take a look at? I think it is as far as being complete/correct for what it has right now. We can open an issue to further investigate using filenos/file objects.

singh-lokendra commented 5 years ago

Yes, upcoming lt-proc arguments would follow similar approach, so it would be good if someone can review this.

sushain97 commented 5 years ago

@TinoDidriksen / @unhammer - would appreciate a second look at this before merging. Particularly on the front of adding the .so/.dll (later) to the releases.

unhammer commented 5 years ago

@sushain97 What do you mean by adding a .so/.dll? I think I'm missing some context here :)

This PR seems to add a hard dependency on swig – should that be behind some --enable-python-api configure switch perhaps? I don't think any other apertiumy project has a hard dep on swig yet. (And any new dependency adds hours of support for those compiling on CentOS, borked macports setups etc.)

Apart from that, LGTM :)

sushain97 commented 5 years ago

Well, I'm not exactly sure what extra effort is required on Tino's end vs. in this PR to build/release the shared library for each combination of Python version and OS.

As for the flag, a great point (and blocker). @Vaydheesh, could you look into adding some flag to that effect?

TinoDidriksen commented 5 years ago

None of the added m4/ax* files are necessary, nor are the new AX_* calls. Plus, I can't use autotools on Windows. https://github.com/hfst/hfst does it only with setup.py, so surely that's doable here as well.

sushain97 commented 5 years ago

@Vaydheesh Is this ready for Tino to take another look at? Also do you need to/have you updated https://github.com/apertium/apertium-python/blob/master/build-swig-wrapper.sh?

singh-lokendra commented 5 years ago

Yes, I have removed the m4 macros and corresponding ax calls.
All of the build process is being done by the setup.py assuming that swig is present in the system path.

Also do you need to/have you updated https://github.com/apertium/apertium-python/blob/master/build-swig-wrapper.sh?

I've already added the --enable-python-bindings flag, so no updates are required

sushain97 commented 5 years ago

@TinoDidriksen does this look better?