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

Added function for reloading module #1040

Merged
merged 6 commits into from
Sep 12, 2017
Merged

Conversation

lathen
Copy link
Contributor

@lathen lathen commented Aug 29, 2017

I tried embedding the interpreter using pybind11 in an application where we want to do on-line processing using Python combined with C++. The processing runs iteratively, so I needed a way to reload the imported modules to allow the user to change the Python code between different runs. This PR simply adds a wrapper to PyImport_ReloadModule() and seems to solve the problem nicely.

If this seems relevant to merge in I can try and finalize it with tests and docs.

I'm not so familiar with the fancy C++ stuff so I'm not sure that the final *this = reinterpret_steal<module>(obj); is good.

@wjakob
Copy link
Member

wjakob commented Aug 30, 2017

Edit: two important things are missing: documentation, and a test case.

@lathen
Copy link
Contributor Author

lathen commented Aug 31, 2017

Yep, will try and finalize this!

@lathen
Copy link
Contributor Author

lathen commented Sep 2, 2017

I've tried setting up a test case to cover this, but I'm not completely happy with it.. I think this change is relevant when importing modules from file, so I basically print a temporary file which I import, call, change, reload and then call again according to the code below. The ugliness is that in order for the pycache to detect that the py file has been updated I had to add a sleep of at least 500ms. What do you think? The whole idea with temporary files is not very pretty so maybe you have a better idea on how to test this. Of course the temp files should be cleaned up properly as well upon test termination.

TEST_CASE("Reload module from file") {
    std::ofstream test_module("test_module_reload.py");
    test_module << "def test():\n";
    test_module << "    return 1\n";
    test_module.close();

    auto module = py::module::import("test_module_reload");
    int result = module.attr("test")().cast<int>();
    REQUIRE(result == 1);

    std::this_thread::sleep_for(std::chrono::milliseconds(500));

    test_module.open("test_module_reload.py");
    test_module << "def test():\n";
    test_module << "    return 2\n";
    test_module.close();

    module.reload();
    result = module.attr("test")().cast<int>();
    REQUIRE(result == 2);
}

@henryiii
Copy link
Collaborator

henryiii commented Sep 2, 2017

Py test has some tools that can help, like setting up a temp directory (or filename, I think). Nevermind, you are in C++ testing (what does PyBind use? That looks like Catch if I had to guess).

Edit (again): I see Catch inside the test embed directory.

If you wanted to test this is in the normal pytests, you could simply put this in a function and then py::print the values and then check them from python.

@lathen
Copy link
Contributor Author

lathen commented Sep 3, 2017

Thanks for the input, and I think it makes most sense to test this from the C++ side with the Catch tests. I pushed a suggested test so you can see where it fits in. The sleep is not very pretty, an option would be to clear the __pycache__ file, but that's only relevant for Python 3 I believe.

@dean0x7d
Copy link
Member

dean0x7d commented Sep 8, 2017

Instead of sleep_for, try the following code to fake the file's timestamp:

auto os = py::module::import("os");
auto stat = os.attr("stat")(module_file);
const auto access_time = stat.attr("st_atime").cast<double>();
const auto mod_time = stat.attr("st_mtime").cast<double>();
os.attr("utime")(module_file, py::make_tuple(access_time, mod_time + 1));

@@ -2,6 +2,9 @@
#include <catch.hpp>

#include <thread>
#include <fstream>
#include <cstdio>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why < cstdio>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought I needed it for std::remove() but it seems to build fine without it.

@dean0x7d dean0x7d mentioned this pull request Sep 10, 2017
@lathen
Copy link
Contributor Author

lathen commented Sep 10, 2017

Instead of sleep_for, try the following code to fake the file's timestamp:
...

I tried this but I believe it has more to do with the timestamp of the cached .pyc file. Changing the timestamp of the .pyc file in the same manner didn't help though so I'm not quite sure about the mechanics behind __pycache__.

However, I think I have a better solution in https://docs.python.org/2/library/sys.html#sys.dont_write_bytecode to avoid the .pyc files altogether. I'll push a new version now.

@henryiii
Copy link
Collaborator

You need to #include <functional> in the test file for GCC7.

@dean0x7d
Copy link
Member

Nice, no more 1 second delay. I didn't know cache file generation could be disabled at runtime. That's very handy.

This looks good to me!

@dean0x7d dean0x7d merged commit c64e6b1 into pybind:master Sep 12, 2017
@dean0x7d
Copy link
Member

Merged, thanks @lathen!

@dean0x7d dean0x7d added this to the v2.2.1 milestone Sep 12, 2017
@samhodge
Copy link

I found my way by virtue of the documentation.

I have an issue where i am trying to call some Python in a C++ plugin within a third party app called Nuke by the company Foundry.

In their Python interpreter I can run the pybind11 pytest suite and everything passes OK.

But if I do something as simple as

py::object ospy = py::module::import("os");

It results in a gdb traceback as follows:

(gdb) bt #0 0x00007fffedfdcdfb in ?? () from /asset/common/software/thirdparty/nuke/11.0v2-build1/arch/linux-any/x86_64/libpython2.7.so.1.0 #1 0x00007fffedfb6821 in PyEval_GetFrame () from /asset/common/software/thirdparty/nuke/11.0v2-build1/arch/linux-any/x86_64/libpython2.7.so.1.0 #2 0x00007fffedfb67dc in PyEval_GetGlobals () from /asset/common/software/thirdparty/nuke/11.0v2-build1/arch/linux-any/x86_64/libpython2.7.so.1.0 #3 0x00007fffedfd2abb in PyImport_Import () from /asset/common/software/thirdparty/nuke/11.0v2-build1/arch/linux-any/x86_64/libpython2.7.so.1.0 #4 0x00007fffedfd135e in PyImport_ImportModule () from /asset/common/software/thirdparty/nuke/11.0v2-build1/arch/linux-any/x86_64/libpython2.7.so.1.0 #5 0x00007fff9848bb89 in pybind11::module::import(char const*) () at /asset/common/software/thirdparty/pybind11/2.2.1-build1/arch/linux-centos6_2/x86_64/python2.7/ucs4/ndebug/include/python2.7/pybind11/pybind11.h:845

Do you have any suggestions on something I could test?

I think I will get in contact with the Foundry about it too.
But given the code path is in the pybind11 realm I thought i would contact the community first.

@samhodge
Copy link

I think this thread may be useful
https://github.com/eklitzke/ptrace-syscall

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

5 participants