haudren / pyparma

Bindings to the parma polyhedra library, allowing to use double description from Python.
8 stars 3 forks source link

Another fork... #1

Open videlec opened 8 years ago

videlec commented 8 years ago

Hello,

I also started a fork of this from Sage videlec.pplpy. Sadly, I I did not know about your project. I fixed several issues with the Sage implementation:

I think it would be better to merge the projects.

I changed the file organization (e.g. moved all external declarations in "ppl_decl.pxd") which creates some incompatibilities between pyparma and pplpy...

Tell me what you think! Best, Vincent

haudren commented 8 years ago

Hello,

Thank you for your work ! It does look like your implementation is cleaner... If I understand well, the only thing that might be missing is the pyparma utils that allow to easily go from cdd H-rep or V-rep to a ppl object.

I think it is a better idea to merge everything into pplpy, even though I liked the pyparma name :(.

If you think this is appropriate, I will open a pull request on your project to add the missing functionality.

Cheers, Hervé

videlec commented 8 years ago

Sure. But what is the usage of this module? How do you go from cdd to here? Anyway, your code should have documentation and doctests (that you can run inside the sphinx-doc repository with "make doctest").

BTW, why did you add the option extra_compile_args=["-fPIC"] to your setup.py?

haudren commented 8 years ago

I encourage you to have a look at what's in pycddlib and or the plain cddlib documentation to understand how the hrep and vrep format works.

Then, the point is to be able to convert directly a numpy array, into either a cdd polyhedron or a pyparma one.

For the tests, I had started to write some in the tests directory, that you can run with nosetests. I don't really like doctests as they clutter the documentation. I encourage you to read them, it will give you an understanding of how this code works, btw this was also taken from sage (but rewritten).

For the fpic option, it is a remnant of a time when I linked this library to something else. It is a good idea to have it when trying to call these functions from other libraries (or c++).

Sorry for the lack of documentation but I had written this module quite quickly and no one had bothered until now to suggest improvements.

videlec commented 8 years ago

Note that I can also make a pull request to pyparma and close pplpy! It is just about the library files and documentation.

I am using docstrings to generate the documentation at http://pythonhosted.org/pplpy/. Nevertheless, I agree that some of the doctests in Sage are useless and would better be in a test module.