Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Aug 29, 2023

Move the private function to the internal C API (pycore_object.h).

Move the private function to the internal C API (pycore_object.h).
@vstinner
Copy link
Member Author

@serhiy-storchaka: I vaguely recall that you considered to expose a similar API to the public C API. Is it still the case? Or what is the PyObject_GetOptionalAttr() API that you added to Python 3.13?

@serhiy-storchaka
Copy link
Member

No, I did not work with _PyType_Lookup(). It seems to me, that _PyType_Lookup() to PyObject_GetAttr() as PyDict_GetItem() to PyObject_GetItem() -- convenient, but flawed by design. And it is not use the descriptor machinery. It is used in many essential parts of the code, and I think that there is a large chance that it is used in third-party projects like Cython. It is difficult if possible to reimplement it with a public API.

Perhaps we should first add a public and better designed variants of _PyType_Lookup() and _PyObject_LookupSpecial() before making them less accessible to third-party developers.

@vstinner
Copy link
Member Author

think that there is a large chance that it is used in third-party projects like Cython

Results of a code search on PyPI top 5,000 projects (2023-07-04).

Affected projects (20):

  • Cython (0.29.36)
  • GDAL (3.7.0)
  • M2Crypto (0.39.0)
  • catboost (1.2)
  • cvxpy (1.3.2)
  • dlib (19.24.2)
  • gevent (22.10.2)
  • mecab-python3 (1.0.6)
  • onnx (1.14.0)
  • onnxoptimizer (0.3.13)
  • onnxsim (0.4.33)
  • osmium (3.6.0)
  • praat-parselmouth (0.4.3)
  • pybind11 (2.10.4)
  • pygraphviz (1.11)
  • python-crfsuite (0.9.9)
  • pythonnet (3.0.1)
  • scipy (1.11.1)
  • sentencepiece (0.1.99)
  • yappi (1.4.0)
@vstinner
Copy link
Member Author

Perhaps we should first add a public

If a private function is widely used and it makes sense to expose it to the public C API, sure, go ahead.

and better designed variants of _PyType_Lookup() and _PyObject_LookupSpecial() before making them less accessible to third-party developers.

Which aspects of the API you would like to change in _PyType_Lookup()?

@serhiy-storchaka
Copy link
Member

Which aspects of the API you would like to change in _PyType_Lookup()?

First of all, it contains PyErr_Clear(). We should get rid of such functions.

If it start returning an error, there are variants of the interface: return a pointer with the following calling of PyErr_Occurred() if it is NULL, or return -1/0/1 and save the reference by provided pointer like in PyObject_GetOptionalAttr(), or return a pointer and set the error flag by provided pointer like in find_name_in_mro() and some other private function. It can also return in what type the name was found if there is a need.

Then you may want to return a new reference, although I am not sure that there is a problem with a borrowing reference here. But we should investigate.

It may be turn out that _PyType_Lookup() is too low level, and that all use cases can be covered by few higher level wrappers. Then we can even remove it from internal API.

@vstinner
Copy link
Member Author

vstinner commented Sep 4, 2023

I close my issue. I prefer to keep _PyType_Lookup() for now.

Well, apparently, it's still interesting to consider adding a public flavor of this API.

@vstinner vstinner closed this Sep 4, 2023
@vstinner vstinner deleted the remove_pytype_lookup branch September 4, 2023 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment