Skip to content

gh-61103: don't use native complex types in ctypes - #133237

Merged
encukou merged 6 commits into
python:mainfrom
skirpichev:no_complex.h/61103
May 5, 2025
Merged

gh-61103: don't use native complex types in ctypes#133237
encukou merged 6 commits into
python:mainfrom
skirpichev:no_complex.h/61103

Conversation

@skirpichev

@skirpichevskirpichev commented May 1, 2025•
edited
Loading

Copy link
Copy Markdown
Member
  • drop _complex.h header
  • use appropriate real arrays to replace complex types

📚 Documentation preview 📚: https://cpython-previews--133237.org.readthedocs.build/

Note, that Py_FFI_SUPPORT_C_COMPLEX check configure check imply support for complex types. So, Py_HAVE_C_COMPLEX check is redundant. Though, I'm not sure if it worth removing.

@skirpichev
pythongh-121249: make complex types available unconditionally in the …
66d7e54
…struct module
Each complex type interpreted as an array type containing exactly two
elements of the corresponding real type (real and imaginary parts,
respectively).
@bedevere-appbedevere-appBot mentioned this pull request May 1, 2025
Closed
@skirpichev

skirpichev commented May 1, 2025

Copy link
Copy Markdown
MemberAuthor

This is on top of #132864

CC @dalcinl

@skirpichev
pythongh-61103: don't use native complex types to implement F/D/G in …
6a20faf
…ctypes
* drop _complex.h header
* use appropriate real arrays to replace complex types
@skirpichev
skirpichevforce-pushed the no_complex.h/61103 branch from efbe0bb to 6a20fafCompareMay 1, 2025 04:32
@skirpichev
skirpichev marked this pull request as ready for review May 1, 2025 05:17
@skirpichevskirpichev changed the title gh-61103: don't use native complex types to implement F/D/G in ctypesgh-61103: don't use native complex types in ctypesMay 1, 2025
dalcinl
dalcinl reviewed May 1, 2025
View reviewed changes
Comment threadDoc/library/struct.rst Outdated
@dalcinl

dalcinl commented May 1, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

@skirpichev Thanks, it looks great. IMHO, and despite your doubts about worthiness, this should be merged. Things are simpler now, you removed lots of code, and a private header.

Minor nit: I think the complex types logic that got added to configure.ac can be safely removed.Needed for ctypes tests.

dalcinl
dalcinl reviewed May 1, 2025
View reviewed changes
Comment threadModules/_struct.c Outdated
@skirpichev@dalcinl
Update Modules/_struct.c
8bbed73
Co-authored-by: Lisandro Dalcin <dalcinl@gmail.com>
encukou
encukou reviewed May 1, 2025
View reviewed changes
Comment threadDoc/library/struct.rst Outdated
Comment threadModules/_ctypes/cfield.c Outdated
@skirpichev@encukou
Apply suggestions from code review
f1fa51b
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@skirpichev
skirpichev requested review from dalcinl and encukouMay 1, 2025 13:39
StanFromIreland
StanFromIreland reviewed May 1, 2025
View reviewed changes
Comment threadMisc/NEWS.d/next/Library/2025-04-25-16-20-49.gh-issue-121249.uue2nK.rst Outdated
@skirpichev@StanFromIreland
Update Misc/NEWS.d/next/Library/2025-04-25-16-20-49.gh-issue-121249.u…
071d57e
…ue2nK.rst
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@encukouencukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 2, 2025
@bedevere-bot

bedevere-bot commented May 2, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit 071d57e 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133237%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 2, 2025
encukou
encukou reviewed May 2, 2025
View reviewed changes

@encukouencukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, merge if the buildbots don't complain.

@skirpichev

skirpichev commented May 2, 2025

Copy link
Copy Markdown
MemberAuthor

@encukou, this pr was on top of #132864, which already tested with buildbots. If we are going to merge this, I would prefer to do it via two separate commits.

@encukou

encukou commented May 2, 2025

Copy link
Copy Markdown
Member

Ah, I missed that note, sorry!
Could you add the latest wording changes there?

@skirpichevskirpichev mentioned this pull request May 2, 2025
Merged
@skirpichev

skirpichev commented May 2, 2025

Copy link
Copy Markdown
MemberAuthor

Could you add the latest wording changes there?

Done. See also minor docs correction in #133249

@skirpichev

skirpichev commented May 2, 2025

Copy link
Copy Markdown
MemberAuthor

Failure in test_frame.py on s390x seems unrelated.

@skirpichev

skirpichev commented May 4, 2025

Copy link
Copy Markdown
MemberAuthor

All failures are related to test_frame.py.

@encukou, I left here only one change, related to the struct module (cosmetic change in _Alignof operands), as it was related to code. I hope it's ok.

@skirpichev

skirpichev commented May 4, 2025

Copy link
Copy Markdown
MemberAuthor

@dalcinl, let us know if you think that linked issue should stay open after merging this.

@dalcinl

dalcinl commented May 4, 2025

Copy link
Copy Markdown
Contributor

@skirpichev I think you can close it. AFAICT, all concerns have been addressed. Many thanks for your hard work in implementing all these changes.

encukou
encukou approved these changes May 5, 2025
View reviewed changes

@encukouencukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, thank you!

@encukou
encukou merged commit 8d0e07e into python:mainMay 5, 2025
@skirpichev
skirpichev deleted the no_complex.h/61103 branch May 5, 2025 09:42
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@skirpichev@dalcinl
@encukou@StanFromIreland@Pranjal095
pythongh-61103: don't use C _Complex types to implement F/D/G in ctyp…
007c71e
…es (pythonGH-133237)
According to the C standard, the memory representation of _Complex types
is equivalent to 2-element arrays. Unlike _Complex, arrays are always available.
- drop _complex.h header
- use appropriate real arrays to replace complex types
Co-authored-by: Lisandro Dalcin <dalcinl@gmail.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@junkmdjunkmd mentioned this pull request Jul 21, 2025
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StanFromIrelandStanFromIrelandStanFromIreland left review comments
@encukouencukouencukou approved these changes
@erlend-aaslanderlend-aaslandAwaiting requested review from erlend-aaslanderlend-aasland is a code owner
+2 more reviewers
@dalcinldalcinldalcinl approved these changes
@DangNhutNguyenDangNhutNguyenDangNhutNguyen approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@skirpichev@dalcinl@bedevere-bot@encukou@StanFromIreland@DangNhutNguyen