-
Notifications
You must be signed in to change notification settings - Fork 6
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
Naming convention for new C API functions #52
Comments
Given this for new API functions why not have a new prefix to clearly distinguish new API functions from old? E.g.
|
A side question of this issue is: How to shrink the size of the C API? See the issue #53 for that. |
Over time, some functions accumulated many variants (versions):
Total: 5 flavors of PyRun_AnyFile(). All of them are still available in Python 3.13! Adding a prefix or a suffix doesn't make a big difference: at the end, we will always need to add a new version in future to fix yet another issue. Someone will come up with a new use case which requires to change an existing API which means adding yet another flavor. So do we prefer accumulating suffixes, prefixes, or both? So far, the trend was to always keep the name short |
Used as follows:
|
IMO changing the suffix should be a one time event and the whole legacy API would still be supported forever anyway: #54 Changing the suffix is an opportunity to reuse existing names for the "sane" behavior. Even if the suffix is changed once, we will still have to add new variants tomorrow. Do you have to agree on a suffix? |
@markshannon @erlend-aasland: are you ok with the Ref suffix convention for new functions returning a strong reference rather than a borrowed reference? |
The struct API sounds appealing. But that would be a 6th version of the same function and so a new name should be found: completely different or add yet another suffix. |
Data point: SQLite simply use a version suffix, increasing the version number for each API revision. Example:
|
What if we have to amend one of the I kind of like the version convention of SQLite; you never know how many variants of an API you'll need, and naming them similar only creates more confusion (in my opinion). For example If |
I think this issue is drifting to a discussion about possible solutions; suggesting to create an issue in the devguide, and continue discussing solutions (that is: guidelines) over there. |
I created python/devguide#1126 for discussion solutions. |
My experience says that yes, for sure, we will have to add a variant of at least one Ref function. In that case, I suggest coming with a new suffix and replace Ref with that suffix. |
In think that Windows not only has Ex suffix but also Ex2. Python naming convention has more diversity in suffixes so it's harder to know which one is the "latest". One issue is that all variants are kept. Usually it's because the new API has a new feature only useful to a minority of users, so most users are fine with the existing API and there is no strong willingness to deprecate/remove old APIs and force a migration to the new ones. The Ref variant is different since it fix the API which was "error prone". The Ref is safer. But. Borrow references are fine in most cases when you use CPython. Even PyPy is fine to return a borrow reference to an item of a container: the pointer remains valid until the container is destroyed. Still, I'm not very motivated to enforce a major migration from PyDict_GetItem() to PyDict_GetItemRef() just to reduce the risk of an unlikely issue. |
An alternative is to just add a number: PyDict_GetItem(), PyDict_GetItem2(), PyDict_GetItem3(), ... I dislike mixing SnakeCase with lowercase "v2". |
The Linux kernel just adds a number suffix to syscalls:
A competitor calling convention exists for Linux syscalls: number of arguments:
There are also 64 bits variants like getdents64(). Linux syscalls: https://chromium.googlesource.com/chromiumos/docs/+/master/constants/syscalls.md |
Yeah, just sticking a number to it is not clear enough, IMO. As you say, there are precedent for using number to imply the number of params, and stuff like 64-bit variants. If you add a (Perhaps we should take this discussion to the "solutions" repo (python/devguide) |
By the way in my list of Python C API suffix, I forgot the number suffix. Some examples:
|
Ok, can we now agree on the versioning of C API calling conventions? 😂
|
It ends up being a variant of the same dilemma; if you need to amend the new API, you'll have to introduce yet another prefix. |
I get the need for version numbers, but given we are overhauling the whole API, wouldn't it make sense to distinguish the new, more principled API from the old, ad-hoc one? |
I think that we are talking past each others here. I see two questions:
I prefer to keep When tomorrow, a new API will be designed, PyDict_GetItem2() will become PyCAPI_PyDict_GetItem() (no suffix): replacing removed PyDict_GetItem() function. In such new API, we will still have to add new functions, we can use the same naming convention (being discussed here). Let's say that all new C API functions must now use a new For a brand new C API, IMO it's more the domain of the issue #54 and I consider that they are unrelated. If the prefix is changed, I would prefer that the whole C API is provided with this prefix. For example, I would feel bad to have to call PyLong_AsLong() after PyAPI_Dict_GetItem(). I would prefer to have PyCAPI_Long_AsLong() included. The problem is that someone has to go through the 1500+ functions of the C API, check if API follows the new guideline, and if yes, introduce an alias:
hint: PyLong_AsLong() doesn't respect the guideline, |
To clarify the discussion, I created issue capi-workgroup/api-revolution#9 to discuss this idea. |
fyi - pep-703 currently spells the
ISTR that in many cases the trailing number was also the number of expected arguments or a similar calling convention notation. But that may have been more by chance (each new API naturally added one?) rather than design. This OS pattern started long before Linux. dup -> dup2 comes from BSD. Regardless, with no clear thing to pick here and nobody wanting to think their "this time it's perfect" API is going to wind up revised in the coming decades... I don't think it matters which naming scheme we pick or even if it winds up wholly consistent. The more important aspect is documentation making it clear which APIs are ideal, old-ideal / reluctantly-supported, and please-never-use-this-legacy status. With the second most important aspect the ability for someone to request only a certain modern version of the API be available via their #include. (similar to how Py_LIMITED_API works?) so their code can't give into wrong API temptations. |
Ok, but so far, nobody approved my PR python/cpython#106005 and @markshannon asked to get a consensus on naming convention before adding the function. So, are you ok with PyDict_GetItemRef() name? What about the others? |
I created python/peps#3182 proposing to formalize the concept of "soft deprecation". It may help to make old functions kept for backward compatibility without the willingness to schedule their removals, but they should not be used for new code.
I proposed issue #54 for that. See also the more radical issue capi-workgroup/api-revolution#9 idea. |
@gpshead I agree that an API we come up with isn't going to perfect, but it will be a lot better than what we have now. Sure, we need to allow for further changes, but version 2s should be rare. The C API should reflect the semantics of the language and features of the VM. While these get added to, they change very rarely. For example, the semantic of getting an item from a dictionary haven't changed, it is just that we are fixing flaws in the current API which doesn't handle errors properly. We have learned that just because a function can't currently fail, we should allow for failure in the API, with a few exceptions: (e.g. Having a clear marker (I prefer a new prefix) in the API that differentiates the old from the new, makes it easy to see which API functions may need updating. |
@iritkatriel added |
I'm not a fan of changing a function name when fixing an issue to avoid adding a suffix. IMO it makes the migration from existing API to the new one more complicated. For me, it's more obvious that PyModule_AddModuleRef() is a replacement for PyModule_AddModule(), and the fact that it has a suffix means that it's the newer API, and there should be there for a reason. If it would be called differently, like PyModule_AppendModule() or PyModule_ModuleAdd(), I would be more confused on what is the old API and what is the new one, which one should I pick? The Ref suffix is a hint that: if you care about avoiding borrowed references, you should check for the list of all new Ref variants and use them in your code. Obviously, if we change the suffix instead, the hint would be even stronger! If a function with new suffix exists, obviously it's a new API and it should be used. But I dislike the idea of changing the suffix since it smells too much like "designing a branch new API" and it's whole new can of worms: see issue capi-workgroup/api-revolution#9. Just one example, in my PR python/cpython#106005 I was asked to change Moreover, in a "brand new API", soon we will have for sure to deprecate API and add new ones. Again, the question of adding a suffix / using a different function name arises. IMO it would feel weird to have to change the suffix each time that we add a variant of a function. |
This is an internal function, so I didn't put too much thought into it. I think we should create a non-internal version at some point. |
At the end, sorry, but I failed to get the preferences of all people involved in this discussion. Do you prefer to switch to the number suffix for new APIs? Do you prefer to keep the status quo: add a name suffix (Ex, Flags, Ref, Object, etc.: the chosen suffix depends on the change)? Currently, the vast majority of new functions use a "name" suffix: only 3 functions use a number suffix (2 suffix for the 2nd version): #52 (comment) The status quo is on the name suffix approach. |
Plese note that adding a suffix means that function has almost the same signature (with possible changing the type of few parameters or returning type or adding new parameters). I think that using names which are only different by a suffix but have completely different signatures would be confusing. Especially if other functions with the same suffix have only only difference (or even only semantic difference) from the original functions. See python/cpython#106005. For functions with radically different signature we should use different naming scheme. |
I created PR #108870 to rename the private Python 3.13 has two functions with an Unchecked suffix:
And one function with Unsafe suffix:
The following functions are also documented as being Unsafe in a comment:
Note: I chose the Fast marker in these PyUnicode function names, and it was a bad idea :-) Nothing proves that it's faster than the public function. It's like METH_FASTCALL, how long before another faster method will happen? :-) |
At least it is better than METH_NEWCALL. |
@serhiy-storchaka @erlend-aasland @encukou @markshannon: Do you have a preference between Unsafe and Unchecked suffix? Question related to python/cpython#108870 addition. |
The words "unchecked" and "unsafe" have different meanings and connotations. In the issue you link to, which is about getting the current thread state or returning |
PyThreadState_Get() and PyInterpreterState_Get() are safe to use, since they are garanteed to be non-NULL. That's what I mean by "safe" vs "unsafe". Well, not sure if calling Py_FatalError() makes it really "safe" since this function aborts the process 😁 Do you have a better suggestion for the name? |
Stick with Unchecked. Just remove the underscore and be done. |
In Rust an
With Rust's interpretation of EDIT: Guido beat me to the reply while I was drafting this comment 😂 |
I would prefer the PyThreadState_GetUnchecked() name proposed by @davidhewitt, instead of the PyThreadState_UncheckedGet() name proposed by @gvanrossum. I like adding suffix: when functions are sorted in their documentation, it's easier to spot the variants. Otherwise, they may be sorted in a "random" order in the doc (unless the alphabetical order is not respected on purpose). Also, IMO it's easier to discover function variants when a suffix is added* to the end of the existing name. I think that I now get the different between "unsafe" and "unchecked". For this function, I agree that "unchecked" is a good fit. |
As you mentioned, Rust's As for |
FYI I updated my PR python/cpython#108870 to rename the function to PyThreadState_GetUnchecked(). So far, nobody proposed a better name. I'm fine with this name. |
Proposed guideline issues: |
Let's continue the discussion in the api-evolution project, in issues listed in the previous comment. |
Are you ok with the naming convention of adding Ref suffix when adding a variant of a function which returns a strong reference instead of a borrowed reference? Do we need to go further in terms of standard?
See also issue #14: "PyLong and PyUnicode don't match the Python type names".
Point raised by @markshannon in my PR adding PyDict_GetItemRef().
(*) I would like to fix the C API to avoid functions returning borrewed references. For that, I add new variants of existing functions which return strong references, by adding a Ref suffix to their name:
In the past, other naming convention were used (other suffixes were added).
(*) To add support for Unicode filenames in the import machinery, I add variants of functions taking
char*
by addingObject
suffix: function takingPyObject*
argument. Examples:(*) Previously, another trend was simply to add
Ex
suffix. Windows API has a similar pattern: WaitForMultipleObjects() => WaitForMultipleObjectsEx(). Examples in Python:(*) A fourth trend (!) is to use WithSomething suffix. Examples:
(*) I also saw another suffix, I don't know if it can be called a convention, a variant of the previous name: add Flags.
So I saw the following conventions to add suffixes (oldest to the most recent):
The funny part is that some added functions get multiple suffixes like PyRun_AnyFileExFlags() (Ex + Flags).
The text was updated successfully, but these errors were encountered: