Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
encukou merged 6 commits intopython:mainfromskirpichev:no_complex.h/61103
May 5, 2025

Conversation

skirpichev
Copy link
Contributor

@skirpichevskirpichev commentedMay 1, 2025•
edited
Loading

  • drop _complex.h header
  • use appropriate real arrays to replace complex types

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

Note, thatPy_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.

…struct moduleEach complex type interpreted as an array type containing exactly twoelements of the corresponding real type (real and imaginary parts,respectively).
@skirpichev
Copy link
ContributorAuthor

This is on top of#132864

CC@dalcinl

…ctypes* drop _complex.h header* use appropriate real arrays to replace complex types
@skirpichevskirpichev marked this pull request as ready for reviewMay 1, 2025 05:17
@skirpichevskirpichev changed the titlegh-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
Copy link
Contributor

dalcinl commentedMay 1, 2025•
edited
Loading

@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 toconfigure.ac can be safely removed.Needed for ctypes tests.

Co-authored-by: Lisandro Dalcin <dalcinl@gmail.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
…ue2nK.rstCo-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@encukouencukou added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMay 2, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@encukou for commit071d57e 🤖

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-buildbotsTest PR w/ buildbots; report in status section labelMay 2, 2025
Copy link
Member

@encukouencukou left a comment

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
Copy link
ContributorAuthor

@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
Copy link
Member

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

@skirpichev
Copy link
ContributorAuthor

Could you add the latest wording changes there?

Done. See also minor docs correction in#133249

@skirpichev
Copy link
ContributorAuthor

Failure in test_frame.py on s390x seems unrelated.

@skirpichev
Copy link
ContributorAuthor

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
Copy link
ContributorAuthor

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

@dalcinl
Copy link
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.

Copy link
Member

@encukouencukou left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@encukouencukou merged commit8d0e07e intopython:mainMay 5, 2025
45 checks passed
@skirpichevskirpichev deleted the no_complex.h/61103 branchMay 5, 2025 09:42
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@StanFromIrelandStanFromIrelandStanFromIreland left review comments

@dalcinldalcinldalcinl approved these changes

@encukouencukouencukou approved these changes

@DangNhutNguyenDangNhutNguyenDangNhutNguyen approved these changes

@erlend-aaslanderlend-aaslandAwaiting requested review from erlend-aaslanderlend-aasland is a code owner

Assignees
No one assigned
Labels
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
[8]ページ先頭
©2009-2025 Movatter.jp