Add Python bindings. #957
Add Python bindings. #957
Conversation
|
This is missing licence header blobs and cmake stuff. |
|
Great! I have two requests:
|
| package_dir={'': 'bindings/python'}, | ||
| packages=setuptools.find_packages('bindings/python'), | ||
| install_requires=_parse_requirements('bindings/python/requirements.txt'), | ||
| cmdclass=dict(build_ext=BuildBazelExtension), |
There was a problem hiding this comment.
Hmm.
Reading up on this, and i think either the cmake build system needs to be explicitly deprecated,
or this should be cmake-based.
There was a problem hiding this comment.
We could have the Python bindings requiring bazel (given they require absl which doesn't have a good cmake solution). No?
Ah... I checked and saw that many files didn't have the copyright / license header, but I've just seen that the public headers do. Which files require it? The published files (the Python in this case), or all of them? What do you mean by CMake stuff? Is is not okay to rely on Bazel for building the Python extension? |
I've added the license headers to the two Python files that will be distributed. Are they needed anywhere else? |
|
If someone wants the python bindings, they need to use bazel, right? i think that's ok for now and if someone needs cmake we/they can always add that. |
Yes, that's correct |
|
On Wed, Apr 15, 2020 at 2:30 PM Chris Jones ***@***.***> wrote:
This is missing licence header blobs and cmake stuff.
Ah... I checked and saw that many files didn't have the copyright /
license header, but I've just seen that the public headers do. Which files
require it? The published files (the Python in this case), or all of them?
Every file, IMO
What do you mean by CMake stuff? Is is not okay to rely on Bazel for
building the Python extension?
I guess i'll just refrain from the review then :)
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#957 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAVUGDGAPXODRLUGHLIRU3RMWLE5ANCNFSM4MH4L3QQ>
.
|
|
Hi Dominic. Apologies for my procrastination on this. I've changed the example to be a |
|
i think it's great. maybe a markdown doc linked from the main README to complete the documentation story would be ideal if you can, but if you want to do that as a followup let me know and i'll land this. |
|
It would be great to get this in. If I can do the documentation in a follow-up, that would be preferable to me. |
* Add Python bindings. * Add license headers. * Change example to a test. * Add example usage to module docstring.
#954