aldanor / ipybind

IPython / Jupyter integration for pybind11
MIT License
66 stars 5 forks source link

ipybind #1

Open aldanor opened 7 years ago

aldanor commented 7 years ago

@wjakob @jagerman @dean0x7d

Hi – tagging you to take a look, what do you think? Cython has a dedicated cell magic, so we should have one (preferably a better one) too :) If you think it would be useful we can provided a link on pybind11's page.

Was quite a bit of pain to make it work properly on Windows, partially due to my inexperience with it and msvc stack in particular, but it looks like things generally work ok. There's a minor amount of things to finish, like adding a few nbval tests, mostly error reporting and stream capturing, documenting a few more things in readme, maybe minor code reshuffling – but most of it is done.

One thing that can be done to is adding a few lines to pybind11's __init__.py, so that one coud do %load_ext pybind11 which is arguably nicer:

def load_ipython_extension(ip):
    try:
        import ipybind
        ipybind.load_ipython_extension(ip)
    except ImportError:
        pass  # or maybe show a warning "ipybind has to be installed for this to work"

(Potential other TODOs I have written down: add a line magic to build external C++ file; allow exporting the module to a user-specified path while figuring the name automatically; expose the building API without wrapping it in IPython's magics, kind of like in cppimport - since there's a bunch of useful stuff here; replace PYBIND11_PLUGIN with PYBIND11_MODULE eventually in the readme)

(It is also possible to compile a big tutorial notebook with various examples from pybind11's documentation and host it in pybind11's repo so users can view it online rendered by github but also run it / play around with it)

dean0x7d commented 7 years ago

This is awesome! I had no idea I wanted something like this, but now I probably won't be able to do without it. It has become my goto for quick tests and I also recently put together a small demo of pybind11 for colleagues. So I definitely like the idea of putting some tutorial notebooks online!

Regarding load_ext in pybind11's __init__.py: I personally think that would be a great addition. The ImportError should definitely have an informative message rather than just pass.

Regarding features: As an option, could the module namespace remain intact? I.e. import m instead of from m import *? I can see both schemes being useful under different circumstances. Also, any plans for automatic cleanup of the build directory?

aldanor commented 7 years ago

@dean0x7d Regarding features: As an option, could the module namespace remain intact? I.e. import m instead of from m import *? I can see both schemes being useful under different circumstances.

Already done (-m), it's just missing from the docs along with a few other things :) (58874a10fcbb1ba2a899c89e09ba4f4d734141cf) There's yet another mode I've planned implementing -- outputting the resulting module to a user specified folder, e.g. current directory, this may be useful if you have other scripts that import your scratch code. It's quite a bit more tricky however, because we don't really know the module name, but could still be done.

I'll get back to finishing documentation and tests soon, and then maybe push to pypi and open a PR on pybind11 to enable load_ext.

aldanor commented 7 years ago

@dean0x7d In regards to cleaning the build folder - do you mean the cached module binaries or the temporary build artifacts (object files etc)? If it's the former, I'm not quite sure, since "caching" and "automatic cleanup" are two opposite things :) Could provide an explicit way to kill everything, e.g. %pybind11_cleanup or the like. Another possibility is some sort of enabling/disabling module caching globally.

dean0x7d commented 7 years ago

In regards to cleaning the build folder - do you mean the cached module binaries?

Yes, those.

Another possibility is some sort of enabling/disabling module caching globally.

That would be nice. The cache dir grows rapidly as the C++ code in the notebook is edited (as far as I understand each change of the code creates a new .so file in the cache) which means that I need to manually delete the cache once in a while. The cache is nice when restarting a notebook, but I think the hit rate is low for REPL.

aldanor commented 7 years ago

That would be nice. The cache dir grows rapidly as the C++ code in the notebook is edited (as far as I understand each change of the code creates a new .so file in the cache) which means that I need to manually delete the cache once in a while. The cache is nice when restarting a notebook, but I think the hit rate is low for REPL.

Hmm, would you have a suggestion for what the most reasonable default would be? You have probably used this package more than myself at this point :) Maybe caching off by default, but possible to turn on globally? Or the other way around? How exactly (and, more importantly, when) should binaries be cleaned up?

One problem is this: you can't remove a binary module on Windows if it's currently imported (you'll get "access denied"). Which means you either have to wait until interpreter shutdown, or do it at start before anything is imported (not sure exactly when). Also, you have to keep hashes (=names) different for different modules because otherwise the module import mechanism gets in the way and you can't reimport stuff.

aldanor commented 7 years ago

Another issue about cleanups I forgot to mention -- there may be multiple kernels open at the same time, so not all cached extensions may be "owned" by just one interpreter.