fix: set __file__ on submodules #5584
Conversation
The docs state ['The caller is responsible for setting a `__file__` attribute'](https://docs.python.org/3/c-api/module.html), but we were not doing that, which interferes with pickling objects in submodules using cloudpickle. Users previously had to set the `__file__` attributes manually, such as in https://github.com/scikit-hep/boost-histogram/blob/1fbbe1632e1665863b9c84b10edf6aa659a14bf1/src/boost_histogram/histogram.py#L83-L90. Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
|
|
@msimacek, |
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
| @@ -1336,6 +1336,11 @@ class module_ : public object { | |||
| if (doc && options::show_user_defined_docstrings()) { | |||
| result.attr("__doc__") = pybind11::str(doc); | |||
| } | |||
|
|
|||
| // GraalPy doesn't support PyModule_GetFilenameObject, | |||
There was a problem hiding this comment.
This is fine if you want to keep it, but the official PyModule_GetFilenameObject() seems most future proof, compared to referencing "__file__" directly.
We have 7 defined(GRAALVM_PYTHON) already under include/pybind11/, I'd lean towards adding another one here.
Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>
Thank you for the notification. I'll add the function for the next version of GraalPy |
|
Yes, if I fix it for GraalPy 25.0 you could do |
include/pybind11/pybind11.h
Outdated
| @@ -1336,6 +1336,19 @@ class module_ : public object { | |||
| if (doc && options::show_user_defined_docstrings()) { | |||
| result.attr("__doc__") = pybind11::str(doc); | |||
| } | |||
|
|
|||
| #ifdef GRAALVM_PYTHON | |||
There was a problem hiding this comment.
| #ifdef GRAALVM_PYTHON | |
| #if defined(GRAALVM_PYTHON) && GRAALPY_VERSION_NUM < 0x190000 |
There was a problem hiding this comment.
Suggestion applied with commit 4682e14
```
/Users/runner/work/pybind11/pybind11/include/pybind11/pybind11.h:1340:32: error: 'GRAALPY_VERSION_NUM' is not defined, evaluates to 0 [-Werror,-Wundef]
^
```
Related ChatGPT conversation: https://chatgpt.com/share/67e6cb99-84b0-8008-99d6-aadc70242cf3
Description
The docs state 'The caller is responsible for setting a
__file__attribute', but wewere not doing that, which interferes with pickling objects in submodules using cloudpickle. Users previously had to set the
__file__attributes manually,such as in https://github.com/scikit-hep/boost-histogram/blob/1fbbe1632e1665863b9c84b10edf6aa659a14bf1/src/boost_histogram/histogram.py#L83-L90.
Suggested changelog entry:
* Set ``__file__`` on submodules.