Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-106320: Remove private _PyType_Lookup() function#108600

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
vstinner wants to merge1 commit intopython:mainfromvstinner:remove_pytype_lookup

Conversation

vstinner
Copy link
Member

@vstinnervstinner commentedAug 29, 2023
edited by bedevere-bot
Loading

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
MemberAuthor

@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?

@vstinner
Copy link
MemberAuthor

@serhiy-storchaka
Copy link
Member

No, I did not work with_PyType_Lookup(). It seems to me, that_PyType_Lookup() toPyObject_GetAttr() asPyDict_GetItem() toPyObject_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
MemberAuthor

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
MemberAuthor

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 containsPyErr_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 ofPyErr_Occurred() if it is NULL, or return-1/0/1 and save the reference by provided pointer like inPyObject_GetOptionalAttr(), or return a pointer and set the error flag by provided pointer like infind_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
MemberAuthor

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.

@vstinnervstinner deleted the remove_pytype_lookup branchSeptember 4, 2023 09:55
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@vstinner@serhiy-storchaka@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp