mjwybrow / adaptagrams

Libraries for constraint-based layout and connector routing for diagrams.
http://www.adaptagrams.org/
257 stars 79 forks source link

dialect::dimensions object's contents not accessible from python swig wrapper #50

Open shakfu opened 3 years ago

shakfu commented 3 years ago

Hi,

The python swig wrapper works great except that I am unable to access the contents of the dimensions objects which is a std::pair<double, double> instance return from DialectNode.getDimensions.

dialect::dimensions is a typedef std::pair<double,double> dimensions

But I am not able to retrieve its contents in python.

In [1]: from adaptagrams import *

In [2]: n = DialectNode.allocate(10.0, 20.0, 30.0, 30.0)

In [3]: d = n.getDimensions()

In [4]: d.first
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-4-6ed88d6b1bba> in <module>
----> 1 d.first

AttributeError: 'SwigPyObject' object has no attribute 'first'

I have tried to add some additional wrapping code to adaptagrams.i. The following gives the same error as above:

%typedef std::pair<double,double> dimensions;
%template() std::pair<double,double>;
%rename(dimensions) dialect::dimensions;

and when I add a %template(dimensions) std::pair<double,double>; it generates the wrapping code but then cannot find it in the resultant extension module.

My knowledge of SWIG is quite limited so would appreciate some advice on how to fix this.

shakfu commented 3 years ago

FYI, I have managed to build an example python3 extension of code with the same signature as dialect::dimensions. I have attached a zip file of two cases: one with and one without the dialect namespace. test_dimensions.zip

There seems to be no need to use %rename as per my earlier assumption.

shakfu commented 3 years ago

Still stuck, If I insert:

%template() std::pair <double,double>;
%template(dimensions) std::pair <double,double>;

into adaptagrams.i, I still get

AttributeError: module '_adaptagrams' has no attribute 'dimensions_first_get'

when I import from anything from adaptagrams.py post-compilation.

skieffer commented 3 years ago

Hi, unfortunately it's been ages since I did anything with SWIG, and it will be a long time before I could give this any serious attention. Hopefully someone else can help. If I do think of anything I will chime in. Good luck!

shakfu commented 3 years ago

Thanks for your reply @skieffer.

It's a really strange issue, since the isolated case seems to work as expected. it may be the case that the order of one of the other declarations is interfering with the expected outcome. Swig isn't being too helpful about this... let's see..

In any case, it's not the end of the world if it isn't resolved since one can read/write from the .tglf format.

shakfu commented 3 years ago

I have asked for advice on this issue over at stackoverflow

jtiai commented 3 years ago

I probably would consider pybind11 instead swig. Slightly more modern solution.

ti 25. toukok. 2021 klo 15.41 Shakeeb Alireza @.***> kirjoitti:

I have asked for advice on this issue over at stackoverflow https://stackoverflow.com/questions/67671475/unable-to-access-a-typedef-defined-stdpairdouble-double-from-its-python-swig

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mjwybrow/adaptagrams/issues/50#issuecomment-847835380, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKORMKRK62TFRFCRQJKNITTPOLHZANCNFSM45KA2SFA .

shakfu commented 3 years ago

@jtiai I'm just a user of the library. (-:

I think the benefit of SWIG is the automated wrapping, Cython, pybind11 and cffi are manual wrapping tools. I've personally only had experience with cython previously but I hear pybind11 is a good option for c++

jtiai commented 3 years ago

I actually have wrapped part of libavoid with pybind11. Wasn't much of a work.

shakfu commented 3 years ago

@jtiai the missing part of the swig interface in this case is really trivial and tiny: just a std::pair<double,double> and it's just ridiculously weird that swig will generate the code but won't enable it. I've moved the declaration all over the adaptagrams.i file without success in case it was an order issue.

Now I'm just irritated about not finding a solution, and having exhausted my own attempts, so I've just posted it on the swig forum in case there's an expert out there who can shed light on this problem: https://sourceforge.net/p/swig/mailman/message/37289720/

S

shakfu commented 3 years ago

I took @jtiai's advice and wrapped up the part I wanted (HOLA in libdialect) using pybind11

@skieffer In case someone else finds this useful, I put the work in a GitHub repo. The pybind11 wrapper for HOLA is in a folder named pybind_adaptagrams

S

shakfu commented 1 year ago

@skieffer @jtiai

As a followup to the above: there is now a project repo for the pybind11 wrapper: pyhola

In practice it seems to perform somewhat consistently with the swig wrapper and holds its own against other algorithms.

skieffer commented 1 year ago

@shakfu, thanks for your work with pyhola! Difficulties often seem to arise with SWIG, so having an alternative integration with Python is great.

Your layout comparison is interesting. I wonder why the adaptagrams and pyhola layouts are different. Different versions of adaptagrams? Different starting positions?

Also the network seems to be (almost?) a tree. Is the "speaker" box connected twice to its neighbor? If so, are ports being used, to model a multigraph? It would be interesting to see how it performed with a network that had more non-tree structure.

shakfu commented 1 year ago

Hi @skieffer

Thanks for your reply.

Your layout comparison is interesting. I wonder why the adaptagrams and pyhola layouts are different. Different versions of adaptagrams? Different starting positions?

The underlying graph is the same but the differences in layout may be because the adaptagrams libs I use to build pyhola are relatively old compared to the current adaptagrams repo state.

This motivated me to add a basic cmake build system which you have commented on in another post.

Also the network seems to be (almost?) a tree. Is the "speaker" box connected twice to its neighbor? If so, are ports being used, to model a multigraph? It would be interesting to see how it performed with a network that had more non-tree structure.

In Max/MSP objects have inlets and outlets. The speaker models a DAC so the two cables from one object's outlet represent a mono cable connecting to left and right inlets of the DAC (which are the left and right audio outputs of the system).

Note that the layout comparison compares only node position layout, edges are not yet considered. I would like progress in that direction but my initial pybind11 wrapper was a bit limited. This pushed me to try to wrap all of the adaptagram libs via an automatic binder generation tool.

This is working surprisingly well, but as before, it makes no sense to do so with old adaptagrams libs... and hence more justification for the cmake project..