Adding Py_PACK_VERSION (original) (raw)

I propose to add the following macros to the limited C API, to make it easier to express and compare versions in the hex format used by PY_VERSION_HEX and Py_LIMITED_API:

In addition I’d like to export library functions with the same names and functionality, for use in wrappers for non-C languages – for example, Python with ctypes.
(The macro-style naming means that we do encourage “serious” wrappers to implement them as compile-time constructs, rather than library calls.)

A draft implementation is in my branch.

brettcannon (Brett Cannon) July 22, 2024, 8:56pm 2

I have to admit I didn’t know what this macro did by the name. My bikeshed calls it Py_VERSION_AS_HEX or something.

vstinner (Victor Stinner) July 23, 2024, 7:21am 3

I’m not sure that the version with 5 parameters is useful. The version with 2 parameters should be enough for most usecases, no?

encukou (Petr Viktorin) July 23, 2024, 7:53am 4

IMO, with any name we choose, if you see it in context you’ll almost certainly know what it does. Once you know it, it’s easier to associate PACK with _bit_-packing.

Anyway, I’m not attached to the name. So, Py_VERSION_AS_HEX and Py_VERSION_FULL_AS_HEX?

In CPython, it’s useful to define PY_VERSION_HEX.
In Cython, micro & release are used relatively often (git grep 0x030 and look for the ones with zeros). Though I’m not sure how many would be there if it was easy to omit them.

Perhaps more importantly, leaving the “full” version out would feel like an omission; as a user I’d wonder why we document the full format but only have a macro for part of it.

vstinner (Victor Stinner) July 23, 2024, 11:50am 5

Cython may want to use its own macro, and I don’t think that we should use Cython as a general use case. Cython is quite specific and special.

ngoldbaum (Nathan Goldbaum) July 23, 2024, 2:21pm 6

For what it’s worth, I was just writing some code that uses Py_VERSION_HEX to decide whether or not to use PyMutex or PyThread_type_lock yesterday. Since the feature was made public in 3.13.0b3, I needed to calculate the hex version and put that in manually. If I had this macro, I think the source code would be left in a more readable state.

da-woods (Da Woods) July 23, 2024, 4:56pm 7

I definitely agree it’s more readable. I’m not sure how much Cython would use it mainly because we’d need to backport it ourselves and right now Py_VERSION_HEX isn’t too hard.

Cython typically gives exact versions for pre-release versions when they’re still in development just to make it easier during the alpha/beta phase. We don’t actually care about supporting old prerelease versions long-term, but just never take the effort to remove the exact versions.

Not sure this is a useful comment with a strong conclusion though

cmaureir (Cristián Maureira-Fredes) July 30, 2024, 7:05am 8

This certainly will help having more readable code, I counted around 40 occurrences where we are comparing raw hex with PY_VERSION_HEX in PySide, however I agree with Victor wrt the simplified version, maybe there is another project that could benefit from the 5 parameters version?

ncoghlan (Alyssa Coghlan) July 30, 2024, 11:23am 9

The fact Py_VERSION_AS_HEX(x, y) is most easily defined as Py_VERSION_FULL_AS_HEX(x, y, 0, 0, 0) seems like sufficient justification for providing the latter macro to me.

And once we’re defining it anyway, it isn’t like sharing it as a public API is going to add an enormous maintenance burden.

That said, I don’t think the “setting Py_VERSION_HEX” use case is super-compelling, as that’s already pretty clear:

#define PY_VERSION_HEX ((PY_MAJOR_VERSION << 24) | \
                        (PY_MINOR_VERSION << 16) | \
                        (PY_MICRO_VERSION <<  8) | \
                        (PY_RELEASE_LEVEL <<  4) | \
                        (PY_RELEASE_SERIAL << 0))

A macro-using version would presumably look something like:

#define Py_VERSION_FULL_AS_HEX(x, y, z, level, serial) \
    ((((x) & 0xFF) << 24) | (((y) & 0xFF) << 16) | (((z) & 0xFF) <<  8) | \
     (((level) & 0xF) <<  4) | ((serial) & 0xF)))

#define Py_VERSION_AS_HEX(x, y) Py_VERSION_FULL_AS_HEX(x, y, 0, 0, 0)

#define PY_VERSION_HEX \
    Py_VERSION_FULL_AS_HEX( \
        PY_MAJOR_VERSION, \
        PY_MINOR_VERSION, \
        PY_MICRO_VERSION, \
        PY_RELEASE_LEVEL, \
        PY_RELEASE_SERIAL \
    )

encukou (Petr Viktorin) September 17, 2024, 1:02pm 10

Getting back to this:

pitrou (Antoine Pitrou) September 17, 2024, 4:09pm 11

If we get Py_PACK_VERSION, then the shorthand Py_PACK_VER is quite useless and inelegant IMHO.

As an additional suggestion, some version check macros Py_VERSION_GT(x, y, z), Py_VERSION_GE(x, y, z), Py_VERSION_LT(x, y, z) and Py_VERSION_LE(x, y, z) would be useful as well.

ncoghlan (Alyssa Coghlan) September 18, 2024, 12:52am 12

In the specific poll given, I voted for the _AS_HEX names.

I would have voted differently if the suggested alternative was Py_PACK_VERSION/Py_PACK_FULL_VERSION.

encukou (Petr Viktorin) September 18, 2024, 8:41am 13

Well, I can’t change the poll, but I think I can append a choice to it here:

ruang (James Roy (RUANG)) October 31, 2024, 11:46am 14

Sorry, I seem to be late. I was thinking about this problem today, and today I published the same proposaland. I actually prefer Py_VERSION for the name, which is concise enough. I looked at Peter implementation, and I personally think it should be implemented as a macro, refer to KERNEL_VERSION in Linux.

this’s my proposaland

encukou (Petr Viktorin) October 31, 2024, 2:23pm 15

Sorry for dropping the ball here.
I’ll assume that whoever voted in both parts of the poll wanted to transfer their vote (as we also hearted Alyssa’s comment). Then, the last option wins, barely:

You can interpret it another way, but it’s pretty close. I’d like to break the tie, and go with that last option. A function-like macro gets a verb:

Py_PACK_VERSION(x, y) & Py_PACK_FULL_VERSION(x, y, z, level, serial)
(edit: corrected signature)

If anyone opposes those, do it now. Otherwise I’ll submit this to the WG next week.

(BTW, if you happen to vote in some democratic election, please demand better poll management!)

These aren’t as general – they assume operand is PY_VERSION_HEX – so Py_PACK_VERSION is needed anyway. I’d like to leave these to another proposal (but we can discuss them here.)

Since PY_RELEASE_LEVEL is never zero, we only need < and >, not also ≤ & ≥.

That would be a great name, but it already has a meaning.

ruang (James Roy (RUANG)) November 1, 2024, 7:08am 16

BTW, if you happen to vote in some democratic election, please demand better poll management!

This election has been quite exciting. However, I believe that initiating a vote on this issue now would be meaningless, as the outcome of the discussion is already clear. My suggestion is that we can directly open an issue in the repository or start one in the capigroup. We should only allow votes of 1, 0, and -1 to indicate agreement, neutrality, or disagreement, don’t discuss whether it’s feasible or any matters unrelated to the name. Once the voting is complete, we can proceed with implementation immediately.
(If the number of votes exceeds 5 people, it’s a relatively high number of votes in the above vote.)

Finally, I prefer Py_PACK_VERSION. It clearly represents the packaging version and doesn’t conflict with other names.

vstinner (Victor Stinner) November 1, 2024, 9:53pm 17

I would prefer to have only Py_PACK_VERSION(x, y). It’s uncommon to have to specify z.

ncoghlan (Alyssa Coghlan) November 2, 2024, 10:01am 18

Having Py_PACK_VERSION align with what’s needed for checks against Py_LIMITED_API makes sense to me.

That’s actually what @encukou suggested the shorthand spelling should mean in the original post, so I suspect the more recent spelling as Py_PACK_VERSION(x, y, z) was just a typo, and it was intended to be Py_PACK_VERSION(x, y) & Py_PACK_FULL_VERSION(x, y, z, level, serial).

pitrou (Antoine Pitrou) November 2, 2024, 8:24pm 19

You might have misunderstood my suggestion? For example, Py_VERSION_GT(3, 10, 1) would return 1 if the Python version is greater than 3.10.1, otherwise 0.

encukou (Petr Viktorin) November 4, 2024, 9:59am 20

Sorry for the typos! I did mean to write Py_PACK_VERSION(x, y) without z, and I definitely didn’t mean to get anyone’s name wrong.

I think I got it right?
I meant to point out that these would only use the current Python version, PY_VERSION_HEX.
If they were added, Py_PACK_VERSION would still be useful for other values in the same format, like Py_LIMITED_API or the PEP 743 macro (provisionally named Py_COMPAT_API_VERSION).