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

PEP 757: reject more ideas #4017

Merged
merged 3 commits into from
Oct 4, 2024
Merged

PEP 757: reject more ideas #4017

merged 3 commits into from
Oct 4, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Oct 3, 2024

  1. drop endianness field of the PyLongLayout struct
  2. keep PyLong_GetNativeLayout() function (was: open question)
  3. reject mpz_import/export-like API

I would appreciate also @zooba opinion on (1). Personally I see no reasons to use non-native endianness for implementation of big integers.

(2) was taken from #3980; it seems nobody from C-API WG objected on extra API call added, so lets keep this as a more convenient interface.

  • Change is either:
    • To a Draft PEP
    • To an Accepted or Final PEP, with Steering Council approval
    • To fix an editorial issue (markup, typo, link, header, etc)
  • PR title prefixed with PEP number (e.g. PEP 123: Summary of changes)

📚 Documentation preview 📚: https://pep-previews--4017.org.readthedocs.build/

* drop endianness field of the PyLongLayout struct
* keep PyLong_GetNativeLayout() function (was: open question)
* reject mpz_import/export-like API

Co-authored-by: Victor Stinner <[email protected]>
peps/pep-0757.rst Outdated Show resolved Hide resolved
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zooba
Copy link
Member

zooba commented Oct 3, 2024

Personally I see no reasons to use non-native endianness for implementation of big integers.

If you'd said "experience has proven that we never need non-native endianness..." then I might have agreed, but my opposition is entirely that we don't know for sure and nobody is willing to pretend that they do.

Better to include the information we have rather than assume it (explicit is better than implicit). At a cost of one field in a probably static structure, I don't see why we wouldn't future-proof ourselves.

@skirpichev
Copy link
Member Author

my opposition is entirely that we don't know for sure and nobody is willing to pretend that they do.

@zooba, but can we have some clue on why someone will want to use non-native endianness for digits? This is not for some storage format, but for an implementation of big integers. Such implementation will be much less efficient than one using some machine integer type (and thus, native endianness) for digits.

I would agreed here with Mark Shannon: "I don’t think endian is unnecessary. Digits are not composed of bytes, but of larger machine integers, so they don’t really have endianness."

Better to include the information we have rather than assume it

This information (endianness) is already available (e.g. via public macros).

The mpz_import/export API have endian parameter just because it's much more generic and can be used to dump/restore mpz_t in binary. But it's not our case.

@zooba
Copy link
Member

zooba commented Oct 3, 2024

but can we have some clue on why someone will want to use non-native endianness for digits?

I can speculate and invent ideas all day long, but I don't want to waste time. For any idea I invent, we can spend ages arguing about that particular idea, but the point is that we don't know for sure. The only argument I will accept is that you somehow do know for sure, because you've exhausted every possible implementation of storing numbers and found that it's absolutely impossible to need non-native endianness. And I'd be skeptical of that claim if you made it, as I'd hope you also would be (and I note that you've never made that claim yet :) )

This information (endianness) is already available (e.g. via public macros).

We're discussing an API that is exported as a function and not necessarily only going to be used by C developers compiling against this particular version of CPython. We cannot assume that public macros are available - certainly not ones that come from the configure script. The limited APIs have to stand alone from that. (This is very much a limited API/stable ABI matter - unstable APIs can rely on macros, because we require that people build their extensions for specific Python versions/platforms in that case. The limited API is not the same.)

@skirpichev
Copy link
Member Author

we don't know for sure

I think we can. If you don't trust me - trust Mark;)

it's absolutely impossible to need non-native endianness.

Only to make implementation more slow. Any other reason of not using machine integer types for "digits"?

@vstinner, I'm sure this extra field is useless. But it's not a big deal. If you feel that removing will block accepting proposed API - let me know, I'll quickly revert that part.

We're discussing an API that is exported as a function and not necessarily only going to be used by C developers compiling against this particular version of CPython.

Mentioned macros are available in compilers (e.g. in gcc you can use check like __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__). So, get platform endianness is not a problem.

And for our application - people even won't do this: mpz_import/export's endian parameter has possible value 0, that means native endianness.

@skirpichev skirpichev changed the title PEP 757: drop endianness & reject more ideas PEP 757: reject more ideas Oct 4, 2024
@vstinner vstinner merged commit 7ccefe1 into python:main Oct 4, 2024
6 checks passed
@vstinner
Copy link
Member

vstinner commented Oct 4, 2024

Merged, thanks.

@skirpichev skirpichev deleted the pep-757-edits branch October 4, 2024 14:01
@skirpichev
Copy link
Member Author

@vstinner, could you please update description of the d.p.o thread? "Abstract" lists absent function, "open questions" section could be also removed.

@vstinner
Copy link
Member

vstinner commented Oct 7, 2024

@vstinner, could you please update description of the d.p.o thread? "Abstract" lists absent function, "open questions" section could be also removed.

Done: https://discuss.python.org/t/pep-757-c-api-to-import-export-python-integers/63895

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

Successfully merging this pull request may close these issues.

3 participants