Skip to content

gh-115231: fill __module__ for built-in class/staticmethods #115232

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.

Sign up for GitHub

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

gh-115231: fill __module__ for built-in class/staticmethods #115232

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9b31877
gh-115231: fill __module__ attribute for built-in staticmethods
skirpichev Feb 9, 2024
be1c7ef
PyObject_GetAttr -> PyObject_GetOptionalAttr
skirpichev Feb 10, 2024
1f854ee
Merge branch 'main' into static-bltin-__module__-115231
skirpichev Feb 10, 2024
c4dec10
fill __module__ attribute for built-in classmethods too
skirpichev Feb 11, 2024
1e25d84
use PyObject_GetAttr & ignore exceptions
skirpichev Feb 11, 2024
63b05a0
Merge branch 'master' into static-bltin-__module__-115231
skirpichev Nov 2, 2024
777bc8d
address review: use type_module()
skirpichev Nov 2, 2024
d1650bf
+ rename news
skirpichev Nov 2, 2024
d0f5bb0
Merge branch 'main' into static-bltin-__module__-115231
skirpichev Nov 2, 2024
e20339b
XXX Revert "address review: use type_module()"
skirpichev Nov 2, 2024
4ba1288
Merge branch 'main' into static-bltin-__module__-115231
skirpichev Feb 24, 2025
543a5b3
Address review: use type_module()
skirpichev Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions Lib/test/test_funcattrs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,29 @@ class BuiltinFunctionPropertiesTest(unittest.TestCase):
# XXX Not sure where this should really go since I can't find a
# test module specifically for builtin_function_or_method.

def test_builtin__module__(self):
import decimal
import math

# builtin function:
self.assertEqual(len.__module__, 'builtins')
self.assertEqual(math.sin.__module__, 'math')

# instance method:
self.assertRaises(AttributeError, getattr, int.to_bytes, '__module__')
self.assertRaises(AttributeError, getattr, decimal.Decimal.exp, '__module__')
self.assertEqual(int.to_bytes.__objclass__.__module__, 'builtins')
self.assertEqual(decimal.Decimal.exp.__objclass__.__module__, 'decimal')

# builtin classmethod:
self.assertEqual(int.from_bytes.__module__, 'builtins')
self.assertEqual(int.from_bytes.__self__.__module__, 'builtins')
self.assertEqual(decimal.Decimal.from_float.__module__, 'decimal')
self.assertEqual(decimal.Decimal.from_float.__self__.__module__, 'decimal')

# builtin staticmethod:
self.assertEqual(bytes.maketrans.__module__, 'builtins')

def test_builtin__qualname__(self):
import time

Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2024-02-10-05-42-26.gh-issue-115231.6T7dzi.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Setup ``__module__`` attribute for built-in class/static methods. Patch by
Sergey B Kirpichev.
6 changes: 5 additions & 1 deletion Objects/descrobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ classmethod_get(PyObject *self, PyObject *obj, PyObject *type)
if (descr->d_method->ml_flags & METH_METHOD) {
cls = descr->d_common.d_type;
}
return PyCMethod_New(descr->d_method, type, NULL, cls);
PyObject *mod = PyObject_GetAttr((PyObject*)type, &_Py_ID(__module__));
Copy link
Member

Choose a reason for hiding this comment

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

This will add an overhead for use of class methods. We should try to find other solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

overhead for use of class methods

You meant for adding static/class methods?

We should try to find other solution.

Unlikely e.g. __name__, it's a member of PyCFunctionObject. I think we can set one only in PyCMethod_New and there is no more cheapest solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PR updated to use type_module().

For classmethods, I doubt there is another solution. Here is a micro-benchmark:

Benchmark ref patch
int.from_bytes 289 ns 446 ns: 1.54x slower

@serhiy-storchaka, does the rest (static methods) looks sane for you? If so, I'll create a separate pr just for that case.

import pyperf
runner = pyperf.Runner()
n = 'from_bytes'
runner.bench_func('int.from_bytes', getattr, int, n)

PyErr_Clear();
PyObject *result = PyCMethod_New(descr->d_method, type, mod, cls);
Py_XDECREF(mod);
return result;
}

static PyObject *
Expand Down
5 changes: 4 additions & 1 deletion Objects/typeobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6653,7 +6653,10 @@ type_add_method(PyTypeObject *type, PyMethodDef *meth)
descr = PyDescr_NewClassMethod(type, meth);
}
else if (meth->ml_flags & METH_STATIC) {
PyObject *cfunc = PyCFunction_NewEx(meth, (PyObject*)type, NULL);
PyObject *mod = PyObject_GetAttr((PyObject*)type, &_Py_ID(__module__));
Copy link
Member

Choose a reason for hiding this comment

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

This might theoretically return NULL and raise AttributeError.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, thanks. Probably we could just clear the error indicator on mod == NULL.

Copy link
Member

Choose a reason for hiding this comment

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

Also take a look at GetOptionalAttr

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 think we want ignore here not just AttributeError's. I did similar fix for class methods and got a failure in the test.test_types.UnionTests.test_or_type_operator_with_bad_module().

Copy link
Member

Choose a reason for hiding this comment

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

It is better to use type_module() for consistency with the rest of this file. It will not give a surprise if Exceptions other than AttributeError should not be ignored.

PyErr_Clear();
PyObject *cfunc = PyCFunction_NewEx(meth, (PyObject*)type, mod);
Py_XDECREF(mod);
if (cfunc == NULL) {
return -1;
}
Expand Down