Repository navigation
C API: Add a replacement for PySys_GetObject #108512
Description
Activity
I propose the name
PySys_GetAttr(). By the way, we already have a private_PySys_GetAttr()function.The existing name
PySys_GetObject()doesn't give any hint on the purpose of the function. It gets an object from where? Which object?By the way, the special Py_TRACE_REFS debug build has a
sys.getobjects()function which is unrelated: it gives the list of all Python objects :-)The name is only the part of the question. Other questions are:
- Should it return a borrowed or new reference?
- Should it raise an exception if
syshas no such attribute and what type of exception?
Currently
PySys_GetObject()returns a borrowed reference and NULL ifsyshas no such attribute. But the code that callsPySys_GetObject()has different reaction on the returned NULL:- Some code raises some exception.
- Some code returns error (and the caller of that code may expect an exception be set, but it is not in this case).
- Some code expects the attribute be optional and does not expect any exception. It would need to silence it if it was raised.
Case 2 looks like a bug. We should raise explicit exceptions in these cases. But what exceptions?
IMO this is similar to the new
PyDict_GetItemRefandPyObject_GetOptionalAttrand should have the same API. That is,int PySys_GetObjectStringRef(char *name, PyObject **result), returning- -1: internal error or API misuse; exception raised
- 0: lookup succeeded; no item was found
- 1: lookup succeeded;
resultset
It gets an object from where? Which object?
Well, from the
sysmodule (seePySys_), and the one associated with the given key :)Reacted by Erlend E. Aasland@serhiy-storchaka: I'm interested to work on such API, but if you prefer to write it, just tell me.
For many years, PySys_GetObject() annoyed me, but I was not motivated enough to change it. For example, I dislike the fact that it returns a borrowed reference. I also dislikes the fact that it returns NULL if there is no attribute and if an error occurs. Also, it never raises any exception, it's bad in many ways :-(
@serhiy-storchaka: I'm interested to work on such API, but if you prefer to write it, just tell me.
I have not started to write a code yet.
There is a similar function
_PyEval_GetBuiltin()which is much more used. It is similar toPyObject_GetAttr()-- returns a new reference or NULL, and always sets error if returns NULL. This interface is more convenient when passed toPy_BuildValue("N").I think that convenient functions for getting an attribute of
sysandbuiltinsshould be in the public API, and they should have the same interface. One of differences is thatPySys_GetObject()takes name as a C string, and it is never used in performance sensitive code, while_PyEval_GetBuiltin()takes name as a Python string, so we perhaps need two variants of each functions.I started to work on preliminary patch and found that
PySys_GetObject()is used so little because more sites use private_PySys_GetAttr()which take a name asPyObject*. So now I have much more cases to consider.We should add PySys_GetAttr() with PyObject name and PySys_GetAttrString() with char* name. It would be consistent with other APIs, no?
Reacted by Petr ViktorinThere are at least 53 sites in CPython code that can use new functions (currently they use
PySys_GetObject(), private functions_PySys_GetObject()and_PySys_GetAttr(), direct access tosysdict,_PyImport_GetModuleAttrString(), etc). Approximately 23 of them treat non-existing object as error, and 29 tolerates the absence. It is approximately because in some sites any errors are silenced, so any variant can be used. 20 sites pass the names as a C string, and 20 sites pass it as a Python string (_Py_ID()). Again, the difference is not strict, because every site that use literal C string can be reritten to use_Py_ID()and vice verse._Py_ID()is faster, but in most cases it does not matter.More detailed, per function:
11
PySys_GetAttr()
12PySys_GetAttrString()
21PySys_GetOptionalAttr()
8PySys_GetOptionalAttrString()All 4 variants are used in almost equal proportions. 4 new functions to replace
PySys_GetObject()may be too much. We can get rid of Python string variants in favor of C string variants at some performance cost (but in most cases the difference is negligible). It is difficult to chose between "optional" and "required" variants, because when we left only one we will need to add eitherPyErr_ExceptionMatches(PyExc_RuntimeError)+PyErr_Clear()orPyErr_SetString(PyExc_RuntimeError, "...")in half of cases.Reacted by Erlend E. AaslandAll 4 variants are used in almost equal proportions. 4 new functions to replace PySys_GetObject() may be too much.
IMO PySys_GetOptionalAttrString functions are too much (adding 2 functions is already "a lot") and not worth it.
If you want to support optional attribute, you should expect
AttributeErrorand ignore it (PyErr_Clear()).IMO PySys_GetOptionalAttrString functions are too much (adding 2 functions is already "a lot") and not worth it.
I read the PR #111035 which replaces
PySys_GetObject()with new functions and I changed my mind. There are many functions which do nothing if asysattribute is missing: having to callPyErr_ExceptionMatches(PyExc_AttributeError)+PyErr_Clear()sounds annoying in this case.- marked [C API] Add PySys_GetAttr() function #129367 as a duplicate of this issue
on Jan 28, 2025 I added the 4 functions to pythoncapi-compat: python/pythoncapi-compat@fde4d34
Feature or enhancement
Proposal:
The
PySys_GetObject()function has two flaws:We need to add a replacement free from these flaws. Any ideas about the API and the name?
Links to previous discussion of this feature:
Linked PRs