Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: python binding/integration #212

Closed
wants to merge 14 commits into from
Closed

Conversation

timkpaine
Copy link
Member

@timkpaine timkpaine commented Aug 26, 2018

  • transfer pypi
  • copyrights
  • sub module?

@timkpaine
Copy link
Member Author

timkpaine commented Aug 26, 2018

ping @texodus and @deepankarsharma for thoughts on how to best do integrations.

@deepankarsharma
Copy link
Contributor

deepankarsharma commented Aug 27, 2018

At first glance the Python binding code looks super clean, nice job! I will do a more detailed review sometime tomorrow.

Some thoughts on how an integration could look like
a. We build all perspective cpp files into a shared library, lets say we call this libpsp
b. We add C++ tests using GTest which links against libpsp and tests the base C++ functionality (lets say we call this target psp_test).
c. We build a Python extension which links against libpsp (lets say pypsp)
d. In the future any other language extensions (Lua anyone?) can link against libpsp.
e. We add some integration tests and benchmark for python that use pypsp and test things end to end.
f. One other thing that might be worth thinking sooner rather than later is to have the notion of "versions" of the C++ code that is loaded for the Python / Javascript extension.

My preference would be for the Python (and future language bindings) to be licensed identically to Perspective under the same license and for the code to be merged into mainline (Rather than via submodules). Would love to hear thoughts from others about this.

@texodus
Copy link
Member

texodus commented Sep 24, 2018

Thanks for the PR!

This looks great but needs some work and planning before we can consider merge (or where even to merge it). I've cleaned it up a bit on the python branch for discussion & review.

I concur with @deepankarsharma on all points above, especially the question of licensing which is a critical consideration.

@texodus texodus closed this Sep 24, 2018
@timkpaine timkpaine deleted the python branch December 10, 2018 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants