Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python.h doesn't expose everything #43

Open
encukou opened this issue May 31, 2023 · 9 comments
Open

Python.h doesn't expose everything #43

encukou opened this issue May 31, 2023 · 9 comments
Labels
evolution-proposed fixable theme: implementation flaws problems with the implementation of specific APIs

Comments

@encukou
Copy link
Contributor

encukou commented May 31, 2023

Some “public” headers, like marshal.h, aren't included from Python.h.

@iritkatriel iritkatriel added fixable theme: implementation flaws problems with the implementation of specific APIs labels May 31, 2023
@erlend-aasland
Copy link

Some headers are intentionally left out, though, like datetime.h.

@vstinner
Copy link
Contributor

vstinner commented Jun 8, 2023

Maybe marshal.h should be moved to the internal C API? Is there any usage of PyMarshal functions outside CPython code base?

@vstinner
Copy link
Contributor

vstinner commented Jun 8, 2023

In Python 3.9, I made some PyFrame functions directly accessible via Python.h, without exposing the whole PyFrameObject structure members directly in Python.h. Previously, #include "frameobject.h" was required.

@davidhewitt
Copy link

Maybe marshal.h should be moved to the internal C API? Is there any usage of PyMarshal functions outside CPython code base?

Someone contributed bindings to marshal a long time ago for PyO3, however I have never personally seen any use of them.

1 similar comment
@davidhewitt
Copy link

Maybe marshal.h should be moved to the internal C API? Is there any usage of PyMarshal functions outside CPython code base?

Someone contributed bindings to marshal a long time ago for PyO3, however I have never personally seen any use of them.

@mattip
Copy link

mattip commented Jun 8, 2023

Biopython uses PyMarshal_WriteObjectToString. With that, I think it is fine for Python.h to not include every public API everywhere and to ask users who wish to use this functionality to add the specific header themselves, as well long as the code is hit in the test_capi tests.

@vstinner
Copy link
Contributor

vstinner commented Jun 9, 2023

If someone uses PyMarshal_WriteObjectToString() and it makes sense, it should be exposed directly by Python.h.

With that, I think it is fine for Python.h to not include every public API everywhere and to ask users who wish to use this functionality to add the specific header themselves

I dislike this API design. I would prefer that Python.h exposes all public APIs. The documentation strongly advices to only #include <Python.h> and nothing else. Over the last years, I removed many header files (usually move them to cpython/ directly, or sometimes to the internal C API). When the header file only contains public functions and it was already included by Python.h, IMO the change was fine according to the documentation which requests to only include Python.h.

Example of removed header files:

  • Python 3.10: symtable.h, ast.h, asdl.h, Python-ast.h, pyarena.h
  • Python 3.11: cellobject.h, classobject.h, code.h, context.h, funcobject.h, genobject.h, longintrepr.h, eval.h

I'm tracking the number of header files at: https://pythoncapi.readthedocs.io/stats.html#file-numbers

@iritkatriel iritkatriel added the v label Jul 21, 2023
@vstinner
Copy link
Contributor

structmember.h is not included by Python.h, but it exposes API which are not prefixed by Py_ prefix. @encukou fixed that in Python 3.10.

errcode.h is not included by Python.h: its E_EOF constant is used by the public PyRun_InteractiveOneObject() function. The document suggests including directly errcode.h. Maybe this constant should be prefixed by Py_ and/or have a better name. It seems like all other constants are only used internally. Maybe the standard C EOF constant should be used instead, or another constant like -1.

@encukou
Copy link
Contributor Author

encukou commented Oct 23, 2023

Proposed guideline issue: capi-workgroup/api-evolution#34

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
evolution-proposed fixable theme: implementation flaws problems with the implementation of specific APIs
Projects
None yet
Development

No branches or pull requests

6 participants