pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

Expose PyEval_EvalCodeEx e.g. as Python::run_ex

Open chris-laplante opened this issue 2 years ago • 11 comments

I have a use-case where I need to evaluate some code and set args. It would be possible by exposing PyEval_EvalCodeEx e.g. as a Python::run_ex method. Or perhaps call it run_with_args?

I am trying to implement it myself for contribution to pyo3, but I am a little stuck on the correct method signature and type conversions. Here is what I have so far. It is mostly just copied from Python::run_code:

fn run_code_ex(
    self,
    code: &str,
    start: c_int,
    globals: Option<&PyDict>,
    locals: Option<&PyDict>,
    args: Option<&PyTuple>,
    kws: Option<&PyDict>,
    defs: Option<&PyTuple>,
    kwdefs: Option<&PyDict>,
    closure: Option<&PyTuple> // TODO no idea what this is. docs say 'a closure tuple of cells.'
) -> PyResult<&'py PyAny> {
    let code = CString::new(code)?;
    unsafe {
        let mptr = ffi::PyImport_AddModule("__main__\0".as_ptr() as *const _);
        if mptr.is_null() {
            return Err(PyErr::fetch(self));
        }

        let globals = globals
            .map(AsPyPointer::as_ptr)
            .unwrap_or_else(|| ffi::PyModule_GetDict(mptr));
        let locals = locals.map(AsPyPointer::as_ptr).unwrap_or(globals);

        let args = args.map(AsPyPointer::as_ptr).unwrap_or(std::ptr::null_mut());

        let code_obj = ffi::Py_CompileString(code.as_ptr(), "<string>\0".as_ptr() as _, start);
        if code_obj.is_null() {
            return Err(PyErr::fetch(self));
        }
        let res_ptr = ffi::PyEval_EvalCodeEx(
            code_obj,
            globals,
            locals,
            args, // ERROR: expecting PyObject *const *args
            0, // TODO
            std::ptr::null_mut(), // TODO
            0, // TODO
            std::ptr::null_mut(), // TODO
            0, // TODO
            std::ptr::null_mut(), // TODO
            std::ptr::null_mut(), // TODO
        );
        ffi::Py_DECREF(code_obj);

        self.from_owned_ptr_or_err(res_ptr)
    }
}

The error at the args line looks like this:

    = note: expected raw pointer `*mut *mut pyo3_ffi::PyObject`
               found raw pointer `*mut pyo3_ffi::PyObject`

It seems like I am passing a pointer to the tuple object, but the method is expecting an array of the tuple elements?

A bit of guidance would be appreciated :)

chris-laplante avatar Apr 06 '22 18:04 chris-laplante

Can you expand on your use case for this? I wonder what the right api for this will be, because I really don't want to have functions with that many arguments.

mejrs avatar Apr 06 '22 19:04 mejrs

Can you expand on your use case for this? I wonder what the right api for this will be, because I really don't want to have functions with that many arguments.

I am trying to (indirectly) run code that is accessing sys.argv[0], but since it's not set I get an exception at runtime. I cannot modify the code.

Personally I only care about args. I'd be fine with setting all the rest to null.

chris-laplante avatar Apr 06 '22 20:04 chris-laplante

I am trying to (indirectly) run code that is accessing sys.argv[0], but since it's not set I get an exception at runtime. I cannot modify the code.

Couldn't you just modify sys.argv directly and put in there whatever you want before running that code?

adamreichold avatar Apr 06 '22 20:04 adamreichold

Couldn't you just modify sys.argv directly and put in there whatever you want before running that code?

Good point. I guess I'd just inject sys.argv = ["whatever"] before running?

chris-laplante avatar Apr 06 '22 20:04 chris-laplante

Still, I don't think it'd be a bad idea to implement this at least partially? One thing that is annoying about the workaround above is that it changes line numbers around.

chris-laplante avatar Apr 06 '22 21:04 chris-laplante

One thing that is annoying about the workaround above is that it changes line numbers around.

We are possibly discussing different things. I did not mean to prepend this to the source, but to modify the sys module in a separate invocation of PyEval. This should persist, shouldn't it? At leat the code below does print ["whatever"].

Python::with_gil(|py| {
    let sys = py.eval("__import__('sys')", None, None).unwrap();
    py_run!(py, sys, "sys.argv = ['whatever']");
});

Python::with_gil(|py| {
    py.eval("print(__import__('sys').argv)", None, None).unwrap();
});

I think as this is state of the sys module, it should stay modified as long as that module is loaded?

adamreichold avatar Apr 07 '22 06:04 adamreichold

Looks like that works, thanks!

I still think there are use cases for implementing a run_ex, don't you? What you gave above certainly works but feels like a hack, especially when the functionality I need is already present in PyEval_EvalCodeEx.

chris-laplante avatar May 09 '22 21:05 chris-laplante

I still think there are use cases for implementing a run_ex, don't you?

Since the workaround is actually more general (being able to express arbitrary preparations instead of just patching up sys.argv), I would actually prefer it. (It can also certainly be written in a nicer fashion using PyModule::import and PyAny::setattr.)

Therefore, I would prefer to not add to PyO3's API surface without a definite requirement.

adamreichold avatar May 10 '22 04:05 adamreichold

For the sake of throwing ideas into the ring, I'd be happy to hear proposals of what an implementation of a builder pattern would look like for this. I've always found the Python::run and Python::eval APIs not immediately obvious in how they differ (takes reading their docs to remember), and there's only py_run! but no py_eval! macro.

A more flexible pattern with extended and clearer functionality would be welcome. Similarly I'm not keen for run_ex as proposed above.

davidhewitt avatar May 10 '22 06:05 davidhewitt

For the sake of throwing ideas into the ring, I'd be happy to hear proposals of what an implementation of a builder pattern would look like for this. I've always found the Python::run and Python::eval APIs not immediately obvious in how they differ (takes reading their docs to remember), and there's only py_run! but no py_eval! macro.

A more flexible pattern with extended and clearer functionality would be welcome. Similarly I'm not keen for run_ex as proposed above.

Sounds good, I'll work on adding a builder. That being said, can you please provide a bit of guidance on the original error message I posted, i.e.:

The error at the args line looks like this:

= note: expected raw pointer `*mut *mut pyo3_ffi::PyObject`
           found raw pointer `*mut pyo3_ffi::PyObject`

It seems like I am passing a pointer to the tuple object, but the method is expecting an array of the tuple elements?

chris-laplante avatar May 10 '22 14:05 chris-laplante

Looks like it should actually be *const *mut PyObject - see #2368.

The easiest way to get one would be to start with &[&PyAny], call .as_ptr() to get *const &PyAny and then cast that to *const *mut ffi::PyObject. (&PyAny is layout equivalent with *mut ffi::PyObject).

davidhewitt avatar May 11 '22 02:05 davidhewitt