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

Should we make it hard for 3rd parties to use private functions? #7

Closed
encukou opened this issue Jan 11, 2024 · 18 comments
Closed

Should we make it hard for 3rd parties to use private functions? #7

encukou opened this issue Jan 11, 2024 · 18 comments

Comments

@encukou
Copy link
Collaborator

encukou commented Jan 11, 2024

In another issue, Guido says:

this problem is almost entirely of our own making since we decided we want to make it hard for 3rd parties to use private functions.

I disagree with the decision. If the “we” is the C API working group, then we have a problem.

This decision goes against the practice of “status quo wins”. It was not done with a PEP, and is dearly missing the Motivation/Rationale sections that would go in a PEP. (FWIW, I've repeatedly asked Victor to write that, as I believe the problem has a better solution -- but can't suggest one if the problem we're solving isn't communicated.)

As the working group, do we stand by this decision?
Would someone care to write down what the decision is and why “we” made it?

@gvanrossum
Copy link

I was being facetious. By "we" I meant Victor, and he started to work on this long before there was a WG. The rest of the core dev team is complicit because they (also "we") didn't push back hard enough. Now that we do have a WG, it's our decision to ratify or roll back. I don't want another discussion about the merits -- we all know where we stand and I doubt that at this point anyone on the WG is likely to change their mind. So let's just put it up for a vote.

@encukou
Copy link
Collaborator Author

encukou commented Jan 12, 2024

I pushed back at every opportunity. I don't know what more I could do.

I'd still like to know what exactly we're voting about. I'm not a good person to write that down.

@gvanrossum
Copy link

I know you did.

I think the proposal is to stop changing the "actual" status of APIs that were considered "private" by name but "public" by exposure. There are two sub-proposals:

(1) We roll back such moves that happened since 3.12.

(2) We keep what was already done in main but don't do any more.

I don't know if there is a difference between the two -- I didn't keep track of what @vstinner changed in main since 3.12 branched off, but I vaguely recall he already rolled back some changes that resulted in failing tests in various 3rd party packages (perhaps reported by Fedora); from this I conclude that there is some difference.

@encukou
Copy link
Collaborator Author

encukou commented Jan 16, 2024

OK. I'd still like to know the reasoning behind the changes -- they seem quite important to @vstinner, and I feel bad pushing back without full understanding

But, well, let's put it to vote:

Don't do any more

If “API” with a leading underscore is public header we should not remove it without reason to do so (e.g. it's blocking a feature or bugfix, it's a security vulnerability, etc.).
That is, we're free to remove/break the (not-quite-)API, but we don't do it proactively.

Roll back such moves that happened since 3.12.

Mark underscored API with Py_DEPRECATED (or _Py_DEPRECATED_EXTERNALLY if still useful in CPython).

[added 2024-02-01]

@vstinner
Copy link

I didn't keep track of what @vstinner changed in main since 3.12 branched off, but I vaguely recall he already rolled back some changes that resulted in failing tests in various 3rd party packages (perhaps reported by Fedora); from this I conclude that there is some difference.

I restored ("reverted") 50 private functions in python/cpython#112026 which lists them.

Removed functions which still need work are being discussed in the meta issue: python/cpython#111481 I'm editing the comment python/cpython#111481 (comment) to keep the list up to date. Most APIs were reverted, but I consider that I would still be nice to have a public API instead of a private/internal API.

In practice, the main remaining ("blocking") API is PyTime which is used by Cython: python/cpython#110850

(Unrelated) Users of the limited C API are also affected by planned removal of deprecated API to configure the Python initialization and get the current configuration: see python/cpython#107954 for details.

@zooba
Copy link

zooba commented Jan 16, 2024

we're free to remove/break the (not-quite-)API, but we don't do it proactively.

I usually vote for this for deprecated functionality as well - remove/break it (after the usual period) if we need to, but don't remove it as soon as possible just because we said it'd be removed one day.

I'm neutral on reverting any changes that have already happened.

@vstinner
Copy link

Private functions are in a gray area. It was discussed to deprecate private functions, rather than removing them directly. Someone suggested to not do that, since it would feel awkward to start documenting a private function just to document that it's deprecated and planned for removal.

@zooba
Copy link

zooba commented Jan 16, 2024

What grey area? The only ambiguous aspect of (some) private functions is whether they are private or not. We can certainly change or remove them whenever we like - it just turns out that we probably shouldn't, and so we're voting to say that we need a better reason than "it's private" in order to remove it.

Don't let my mention of deprecation lead the conversation astray. It's a side note to my +1 vote, not directly relevant to this topic.

@encukou
Copy link
Collaborator Author

encukou commented Jan 17, 2024

Not sure what discussion you're referring to, but I was thinking about adding Py_DEPRECATED (or _Py_DEPRECATED_EXTERNALLY for internals that are still useful), to try catching users' attention before removal.

Still, IMO it should only be done after a replacement is in place, or if we know of no users. Or if it blocks development, of course.

@nitzmahone
Copy link

nitzmahone commented Jan 30, 2024

adding Py_DEPRECATED (or _Py_DEPRECATED_EXTERNALLY for internals that are still useful), to try catching users' attention before removal

it should only be done after a replacement is in place, or if we know of no users

For CFFI (a frequent consumer of private APIs), +1 to this approach. Of course we should be using public APIs wherever reasonable/feasible, but sometimes there's no alternative. These kinds of breakages are just "the cost of doing business" this way, but if these suggestions were applied consistently, I'd make sure our canary builds for pre-release Pythons always treat those deprecation warnings as errors. Failing-fast would clearly surface problems like this at build-time, rather than trying to diagnose failed symbol imports at runtime (which can masquerade as all sorts of other problems, depending on the importing code).

@encukou
Copy link
Collaborator Author

encukou commented Feb 1, 2024

OK, I added that option to the vote comment.

@gvanrossum
Copy link

Mark underscored API with Py_DEPRECATED (or _Py_DEPRECATED_EXTERNALLY if still useful in CPython).

I don't think we need to.

@vstinner
Copy link

vstinner commented Feb 4, 2024

Should we make it hard for 3rd parties to use private functions?

IMO such question is misleading. My goal is to promote the limited C API and the stable ABI and to slowly move everybody there and make it the default. The usage of private functions is a blocker issue for such goal, since private APIs are not part of the limited C API.

The bet here is that we are very close from being be to use the limited C API by default, there are only a few remaining issues which can be fixed. The question is more the willingness to address these issues.

There are very specific use cases where internal functions should be usable outside Python, but it should be the exception, not the default.

If Cython, numpy, lxml, Pillow, etc. use a private function, we must provide a public documented and tested API.

Don't do any more
Roll back such moves that happened since 3.12.
Mark underscored API with Py_DEPRECATED (or _Py_DEPRECATED_EXTERNALLY if still useful in CPython).

I disagree with these 3 options. None are going to the direction of a "better public API". I don't see how not doing anything is going to make the situation better on the long term.

Is your goal to make sure that the C API doesn't change anymore? PEP 733 "An Evaluation of Python’s Public C API" has a long list of issues: https://peps.python.org/pep-0733/ How can we fix them if the C API doesn't change anymore?

@zooba
Copy link

zooba commented Feb 5, 2024

The goal of those first two questions is to stop the changes and talk about it. If we can agree to do that soon enough, we might be able to get changes into 3.13 still, but the longer we drag out the "should we design the API or just change it" question the less likely that becomes.

The deprecation question is one of the things to discuss, but it's not critical right now.

@gvanrossum
Copy link

Since we started an internal (to the C API WG) discussion thread about our long-term vision, I propose to deprioritize this question for now until we've come to an agreement on the WG's vision.

@vstinner
Copy link

"Should we make it hard for 3rd parties to use private functions?" question scope is quite broad.

I created the issue Keep alias or not when replacing a private API with a public API? Action: add guidance in the devguide to have a more concrete action. It should be easier to decide on this more narrow scope.

@vstinner
Copy link

Can we close this issue?

@gvanrossum
Copy link

Let's close. I think our current stance is not to remove any more private-looking APIs that are publicly available (i.e., without defining things like Py_BUILD_CORE).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants