python / cpython

The Python programming language
https://www.python.org
Other
63.54k stars 30.44k forks source link

"DeprecationWarning: builtin type has no __module__ attribute" when using PyStructSequence_NewType from a module method #124182

Closed ffelixg closed 1 month ago

ffelixg commented 2 months ago

Bug report

Bug description:

Below is an example producing the warning. This warning does not occur when the type is created in the module's init function. When using PyStructSequence_InitType2, the resulting type gets "builtin" as the __module__ attribute and there is no deprecation warning. It's also possible to add a field named __module__ to prevent the warning. Setting the __module__ attribute after creating the type does not help, the warning gets raised inside the PyStructSequence_NewType function.

I couldn't find anything indicating that I'm not supposed to use PyStructSequence_NewType in this way.

#include <Python.h>

// the type is usable, but we get: <string>:1: DeprecationWarning: builtin type TypeName has no __module__ attribute
// python3 -c "import c_ext; c_ext.create_type()"

static PyObject* create_type(PyObject* self) {
    PyStructSequence_Field fields[] = {
        { "FieldName", "docs" },
        { NULL, NULL },
    };
    PyStructSequence_Desc desc = {"TypeName", "docs", fields, 1};
    return (PyObject*)PyStructSequence_NewType(&desc);
}

static struct PyMethodDef methods[] = {
    {"create_type", (PyCFunction)create_type, METH_NOARGS},
    {NULL, NULL}
};

static struct PyModuleDef module = {
    PyModuleDef_HEAD_INIT, "c_ext", NULL, -1, methods
};

PyMODINIT_FUNC PyInit_c_ext(void) {
    return PyModule_Create(&module);
}

CPython versions tested on:

3.10, 3.11, 3.12, 3.13

Operating systems tested on:

Linux

Linked PRs

rruuaanng commented 2 months ago

I'm following this issue,maybe you can awaiting message for me.

rruuaanng commented 2 months ago

I'm not totally sure if this was the original intention, but it seems like PyStructSequence_Field was meant to let users define module attributes, which in turn would help determine the name of the user-extended modules."

rruuaanng commented 2 months ago

If that's not how it's supposed to work, I can totally put together a PR to sort it out.

rruuaanng commented 2 months ago

The warning is triggered by r = PyDict_Contains(dict, &_Py_ID(__module__))https://github.com/python/cpython/blob/4420cf4dc9ef7bd3c1c9b5465fa9397304bf0110/Objects/typeobject.c#L5068 When r == 0 and there’s no secondary reference, it raises this warning. To get rid of the warning, you could add a __module__ entry when it’s zero, but the value for that key-value pair seems uncertain and makes the rest of the logic pointless. https://github.com/python/cpython/blob/4420cf4dc9ef7bd3c1c9b5465fa9397304bf0110/Objects/typeobject.c#L5069-L5092 I think this is normal behavior, and maybe I should ask a core developer about it.

rruuaanng commented 2 months ago

cc: @brettcannon @vstinner

vstinner commented 2 months ago

DeprecationWarning: builtin type has no module attribute" when using PyStructSequence_NewType from a module method

It's an issue in your extension. The warning means that the type name has no module name. You should replace:

    PyStructSequence_Desc desc = {"TypeName", "docs", fields, 1};

with:

    PyStructSequence_Desc desc = {"c_ext.TypeName", "docs", fields, 1};

where c_ext is your extension name.

rruuaanng commented 2 months ago

DeprecationWarning: builtin type has no module attribute" when using PyStructSequence_NewType from a module method

It's an issue in your extension. The warning means that the type name has no module name. You should replace:


    PyStructSequence_Desc desc = {"TypeName", "docs", fields, 1};

with:


    PyStructSequence_Desc desc = {"c_ext.TypeName", "docs", fields, 1};

where c_ext is your extension name.

@ffelixg

hugovk commented 2 months ago

Hi @rruuaanng, please could you try and make fewer comments where they don't add much to the discussion? For example, there's no need to quote Victor's last comment and only ping the OP - it's safe to assume they're subscribed to their own issue. Thanks.

ffelixg commented 2 months ago

Fair enough, thanks. I did not think that the type name was more than aesthetics. The only question that remains then is why InitType behaves differently (better?) with it defaulting to builtins (although still not including it in the type's repr).

It might also be helpful to mention that the module name must be included in the documentation for the name field.

If you think it's fine the way it is then this can be closed.

vstinner commented 1 month ago

PyStructSequence_InitType() and PyStructSequence_InitType2() are legacy APIs for static types, you should use PyStructSequence_NewType() for heap types.

vstinner commented 1 month ago

It might also be helpful to mention that the module name must be included in the documentation for the name field.

Does someone want to propose a PR for that? The function is documented in Doc/c-api/tuple.rst.

ffelixg commented 1 month ago

Does someone want to propose a PR for that? The function is documented in Doc/c-api/tuple.rst.

The rules seem to be similar to this, so I took that as a base, removed the parts that deal with types that don't belong to a module and mentioned that a dot must be included.

vstinner commented 1 month ago

@ffelixg adjusted the doc in change https://github.com/python/cpython/commit/3287c834e5370294e310450115290979aac06efa. I close the issue.