aphearin / package-template

Template for Astropy affiliated packages. Maintainer: @astrofrog
0 stars 0 forks source link

C extension builds but is not discoverable #1

Open aphearin opened 9 years ago

aphearin commented 9 years ago

@astrofrog - I'm working on a minimal example for the package template demonstrating the Astropy style for how to include a C extension into an affiliated package. I have been basing this branch by following the Astropy Developer Docs and modeling the organization after Astropy source code. I think something along these lines could be instructive for users such as myself who have little-to-no experience with writing C extensions.

What I've done is super simple. I've just gone into example_subpkg and created a src directory and an include directory, and placed a trivial C function and header file in those locations, respectively. Then I wrote a setup_package.py file modeled according to the Developer Docs. That's it.

I must be making a very silly noob mistake because my code builds, but when I import the sub-package, the _customadd method is not available to be called. Can you offer some guidance?

If you think including this minimal example is not such a good idea, that's ok, too. Either way, I really want to understand how to write wrappers in the Astropy-way, so your help would be appreciated even if this branch is just going to get junked.

astrofrog commented 9 years ago

@aphearin - can you point me to the branch where you are doing this so I can try it out?

aphearin commented 9 years ago

Sure thing. The branch is called cext-example. Thanks in advance.

aphearin commented 9 years ago

@astrofrog - I just got an email from @eteq that set me straight on what I was missing. I figured it was something noob-ish, and it was. Why don't you just ignore this and let me take another crack at this after following his recommendation to use cython (copied below). Sound good?


from @eteq:

... There are two ways to easily get around this that I know of. One is to use the ctypes library in the python stdlib. That lets you interface between python and C level types. so you'd write a python module with a function that uses ctypes to first load the C library, and then call it (following so particular set of rules for converting python types to C and back again). So that never uses the Python C API.

The other option is my preference: use cython. You can write a .pyx file that has a function with the python-level calling semantics, but at the top you do something like cimport minimal_cext. Then inside the function you just do something like return minimal_cext.function(pyarg1, pyarg2). Add the .pyx file to your extension, and then you should get what you want. Basically Cython just did all the boilerplate work I mentioned above for you.

aphearin commented 9 years ago

Or, if you prefer, I can just close this Issue if you have no interest in providing a minimal example of how to wrap C code in the package-template. Just let me know either way.

eteq commented 9 years ago

In case it's not clear, I'm very much in favor of such an example. Whether it's in the template itself or in the docs for the template is less clear to me, but this has come up often enough that I think it would be useful to show it pretty clearly.

aphearin commented 8 years ago

So I've implemented the cython wrapping style that @eteq suggested. The one additional piece is that I added a pxd file for my declarations - I'm not sure how you do your C wrapping without this. But with the cython wrapper, the code again compiles and shows all signs of having been successfully wrapped. Yet, when I import example_subpkg, the custom_add function is still not discoverable. So I think I have made progress in that cython is now doing the wrapping, but something about the setup file is not correct. Would you mind having a look @astrofrog or @eteq ?

eteq commented 8 years ago

@aphearin I did python setup.py build in your cext-example branch, and then I did:

cd build/lib.macosx-10.10-x86_64-2.7
python 
>>> from packagename.example_subpkg import simple_cython_wrapper
>>> simple_cython_wrapper.cython_wrapped_custom_add(1,2)
3

so isn't that what's supposed to happen? Or you want it to be discoverable directly from example_subpkg?

aphearin commented 8 years ago

@eteq - Yes, that is what's supposed to happen for a system-wide install. That's why I was reasonably confident that my wrapper was written correctly. But when I do the following:

$ cd [packagename-root]
$ python setup.py build_ext --iplace
>>> from packagename import example_subpkg

Then example_subpkg has no methods bound to it. This is one of the features of cython code in a package that I find a bit baffling and that I want to write documentation for (as soon as I understand it, that is).

eteq commented 8 years ago

@aphearin - ohhh, I get it, and I see I see the issue. I can do this, which I think is what you want:

python setup.py build_ext --iplace
python 
>>> from packagename.example_subpkg import simple_cython_wrapper
>>> simple_cython_wrapper.cython_wrapped_custom_add(1,2)
3

There's a subtle difference there from what you were expecting: you wanted from packagename import example_subpkg to have the function you defined in simple_cython_wrapper.pyx. But it won't, because the name you've set the Extension to (in setup_package.py) is packagename.example_subpkg.simple_cython_wrapper. So it works just like an ordinary module: if it were just a .py file instead of a Cython extension, you'd have to import it like what I showed above, and extensions work just the same way.

So to get what I think you want, you should just change packagename/example_subpkg/__init__.py by removing the existing __all__=... line and instead have it say from .simple_cython_wrapper import *. Then you should be able to do example_subpkg.cython_wrapped_custom_add(1,2)

aphearin commented 8 years ago

Eureka!

aphearin commented 8 years ago

The version of this branch I just pushed includes minor adjustments and cleanups, and also a README I have written that explains various conventions of using Cython to build a C extension within the package-template framework.

So obviously this was my first ride on the bull at the C-wrapping rodeo: I was starting from an entirely clean slate of a knowledge base. Since I don't know who the target audience is for the package-template, I don't know whether the README I wrote, or possibly even this entire example, is too basic for what y'all intended. If that's the case, it's perfectly fine by me - please don't hesitate to deem this material too noob to document. At the very least, this has been a highly instructive exercise for me.

For whatever it's worth, here is my testimonial. I think this branch and README, or something very much like it, are indeed in the spirit of the package-template. When I was starting out developing Halotools about a year ago, I was more or less overwhelmed with the task of simultaneously learning python package structure conventions from scratch, together with learning python from scratch. Having the package-template as a base to start from was extremely helpful, because I could simply pattern-match until I got the hang of relative imports, calls to a setup file, and so forth. My intention for this branch is to provide a documented pattern that other new developers can use immediately, so that if they have existing C code that they want to include in their package, they can similarly pattern-match until they get the hang of it. It seems exceedingly likely that there will be future users of the package-template who have experience in python and C, but no experience in wrapping. If such users are part of the intended audience of the package-template, then I think something like this is warranted.

The main drawback of this that I see is that there are actually many, many other variations on how to do the wrapping. This is just one example of how to wrap a single function that already has a header file written for it. I would happily learn the ropes of another use-case, and write that up as well, if that were considered necessary. But the thing is, I think it is precisely because there are so many different use-cases documented on cython.org and elsewhere, that it creates a needlessly large overhead learning curve to just doing a simple wrapping exercise like this one. Just like the package-template provides a bare-bones package structure that users can build upon, the wrapper code and documentation in this branch provides an introduction to the material that is both gentle and also covers a reasonably wide range of functionality. Or at least that is my intention. Let me know what y'all think at your leisure.

cc @astrofrog @eteq

eteq commented 8 years ago

I'm in favor of including this for just the reasons you gave, @aphearin. So I'm ok with you just issuing a PR with this branch to astropy/package-template. @astrofrog, do you agree?

Note, though, that you'll want to issue a PR to the main astropy repository to edit https://github.com/astropy/astropy/blob/master/docs/development/affiliated-packages.rst, though: just mention the existence and purpose of this change and that the details are in the subpackage README. You might also have to edit the checklist to make sure it includes instructions on renaming/removing where appropriate (although it might not need any changes in that area, I'm not sure offhand).