Skip to content

Commit

Permalink
try harder to resolve __module__ from metaclass parents.
Browse files Browse the repository at this point in the history
  • Loading branch information
t-kalinowski committed Oct 28, 2024
1 parent 7566e0c commit e1c159e
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 6 deletions.
29 changes: 25 additions & 4 deletions src/python.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,18 @@ bool was_python_initialized_by_reticulate() {
namespace {
const std::string PYTHON_BUILTIN = "python.builtin";

class ScopedAssign {
bool& flag;
bool oldValue;
public:
explicit ScopedAssign(bool* f, bool newValue) : flag(*f), oldValue(*f) {
flag = newValue;
}
~ScopedAssign() {
flag = oldValue;
}
};

std::string get_module_name(PyObject* classPtr) {
// Can't throw exceptions here, since we hit this while fetching exceptions.
PyObject* moduleObj;
Expand Down Expand Up @@ -612,12 +624,21 @@ std::string get_module_name(PyObject* classPtr) {
return NULL;
}

// Fallback, if type(class) != type (i.e., it's a metaclass),
// try to to fetch type(class).__module__. But make sure we're not recursing more than once
static bool recursing = false;
if (!recursing && !PyType_CheckExact(classPtr)) {
// get_module_name is marked as noexcept, so recursing = false is guaranteed to be executed
auto _recursion_guard = ScopedAssign(&recursing, true);
return get_module_name((PyObject*) Py_TYPE(classPtr));
}

// if (PyErr_Occurred()) PyErr_Print();
// REprintf("__module__ not a string\n");
// fallback for when __module__ is not a python string.
// E.g., a property obj of a metacalss (wrapt.ProxyObject)
// We can maybe try harder if type(class) != type and fetch
// type(class).__module__.
// fallback for when __module__ is not a python string, or resolvable
// from type(cls).__module__.
// Note, we don't want to throw an exception here, as this is a hot code path
// excercised heavily when already handling exceptions.
return "";
}

Expand Down
9 changes: 7 additions & 2 deletions tests/testthat/test-python-objects.R
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,13 @@ assert isinstance(Dict({}), dict)
# '__reduce__'?
# - Note: dict.__module__ still returns 'builtins', the above Exceptoin is a
# metaclass/ObjectProxy issue
# - We do the best we can and build an R class vector as:
# classes <- c("Dict", "ObjectProxy", "NewBase", "python.builtin.object")
# - So for metaclasses like this we do the best we can and try to resolve
# __module__ from the type(type(cls)) and build an R class vector as:
# classes <- c("wrapt.wrappers.Dict", "wrapt.wrappers.ObjectProxy", "wrapt.wrappers.NewBase",
# "python.builtin.object")
# (we could instead fail earlier and instead build:
# classes <- c("Dict", "ObjectProxy", "NewBase", "python.builtin.object")
# it's not clear which is a better approach)

expect_true(grepl("Dict$", classes[1]))
expect_true(grepl("ObjectProxy$", classes[2]))
Expand Down

0 comments on commit e1c159e

Please sign in to comment.