Skip to content

gh-61103: support double complex (_Complex) type in ctypes #120894

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

Merged
merged 57 commits into from
Jul 1, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Jun 23, 2024

Example:

>>> import ctypes
>>> libm = ctypes.CDLL('libm.so.6')
>>> libm.clog.argtypes = [ctypes.c_double_complex]
>>> libm.clog.restype = ctypes.c_double_complex
>>> libm.clog(1+1j)
(0.34657359027997264+0.7853981633974483j)

ctypes.c_double_complex is available only if compiler does support complex arithmetic (Annex G).


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



Notes for reviewers:

  • It seems, most compilers implement this optional feature of C11+. (Though neither does this correctly.) Maybe we should require one?
  • only one complex type was added; if this looks good - I can add the rest (float complex and long double complex) in this pr or in other.
  • c_double_complex._type_ chosen to be "C". Maybe we should allow instead multiple chars to specify types (e.g. "Cd" for double complex)?
  • this pr doesn't include changes in the struct module. Perhaps, I should add support for complex type here as well?
  • if test_complex_round_trip() does make sense for reviewers, here is a simple refactoring to avoid adding yet another assertFloatsAreIdentical() helper: gh-121039: add Floats/ComplexesAreIdenticalMixin to test.support.testcase #121071

Example:
```pycon
>>> import ctypes
>>> ctypes.__STDC_IEC_559_COMPLEX__
1
>>> libm = ctypes.CDLL('libm.so.6')
>>> libm.clog.argtypes = [ctypes.c_double_complex]
>>> libm.clog.restype = ctypes.c_double_complex
>>> libm.clog(1+1j)
(0.34657359027997264+0.7853981633974483j)
```

``ctypes.__STDC_IEC_559_COMPLEX__`` is ``0`` if compiler doesn't support
complex arithmetic (Annex G).
skirpichev and others added 2 commits June 23, 2024 09:00
Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
@skirpichev skirpichev requested a review from nineteendo June 23, 2024 09:36
Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
@picnixz
Copy link
Member

picnixz commented Jun 23, 2024

I can add the rest (float complex and long double complex) in this pr or in other.

I'd prefer having a separate PR because we might need aliases in case sizeof(long double) == sizeof(double) (the same technique that is used for long long and long IIRC)

c_double_complex.type chosen to be "C". Maybe we should allow instead multiple chars to specify types (e.g. "Cd" for double complex)?

I think it might be indeed a good alternative, because otherwise you need to find good 1-char names for float complex and long double complex in the future, but I think Cf, Cd and Cl (or Cq for quad-precision) are good enough. But for that we need to update the fielddesc structure with an additional field that would be a "modifier" of some sort or allow the "code" to be more than just a single character.

this pr doesn't include changes in the struct module. Perhaps, I should add support for complex type here as well?

I'd say in a separate PR since there is a translation of ctypes format to PEP 3118 names (where complex numbers are represented by 'Z').

skirpichev and others added 2 commits June 23, 2024 14:47
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@skirpichev skirpichev force-pushed the ctypes-c_complex-61103 branch from 45f8544 to 013765e Compare June 23, 2024 13:07
@skirpichev skirpichev force-pushed the ctypes-c_complex-61103 branch from 013765e to f9c709c Compare June 23, 2024 17:33
@nineteendo
Copy link
Contributor

Use a force-push only to fix typo's or revert a merge.

@skirpichev
Copy link
Member Author

Use a force-push only to fix typo's or revert a merge.

Thank you for advice. Usually I don't do this. I hope this time force-push was OK, as it's restricted to unreviewed commits.

@nineteendo
Copy link
Contributor

nineteendo commented Jun 23, 2024

It's less bad if no-one commented on those commits, but it's still more work to figure out what you changed.
It's also more distracting than a regular commit. (also, you don't need to quote the whole previous message)

* add configure test for _Complex type
* define HAVE_C_COMPLEX
* move workarounds for buggy implementations to Module/_complex.h
@vstinner
Copy link
Member

!buildbot s390x SLES

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit e9cab4a 🤖

The command will test the builders whose names match following regular expression: s390x SLES

The builders matched are:

  • s390x SLES PR

@skirpichev
Copy link
Member Author

skirpichev commented Jun 30, 2024

Well, one failure seems to be related:
https://buildbot.python.org/all/#/builders/1262/builds/13/steps/3/logs/stdio
Edit (similar one):
https://buildbot.python.org/all/#/builders/855/builds/119

Looking on the libffi sources, it seems that FFI_TYPE_COMPLEX is available unconditionally, even if platform doesn't support interfaces to _Complex (for sparc it works since v3.3). We should check FFI_TARGET_HAS_COMPLEX_TYPE instead.

I'll update PR when build finish.

@vstinner
Copy link
Member

!buildbot AMD64 Fedora Rawhide PR

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 6d5bf66 🤖

The command will test the builders whose names match following regular expression: AMD64 Fedora Rawhide PR

The builders matched are:

  • AMD64 Fedora Rawhide PR

@vstinner
Copy link
Member

!buildbot AMD64 Fedora Rawhide PR

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 6d5bf66 🤖

The command will test the builders whose names match following regular expression: AMD64 Fedora Rawhide PR

The builders matched are:

  • AMD64 Fedora Rawhide PR

@vstinner
Copy link
Member

Oh, Rawhide failure is unrelated: "No space left on device".

@vstinner
Copy link
Member

!buildbot PPC64LE CentOS9 PR

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 6d5bf66 🤖

The command will test the builders whose names match following regular expression: PPC64LE CentOS9 PR

The builders matched are:

  • PPC64LE CentOS9 PR

@vstinner
Copy link
Member

!buildbot PPC64LE Fedora Stable Clang PR

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 6d5bf66 🤖

The command will test the builders whose names match following regular expression: PPC64LE Fedora Stable Clang PR

The builders matched are:

  • PPC64LE Fedora Stable Clang PR

@vstinner vstinner added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 1, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @vstinner for commit 6d5bf66 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 1, 2024
@vstinner vstinner merged commit 6988ff0 into python:main Jul 1, 2024
78 of 90 checks passed
@vstinner
Copy link
Member

vstinner commented Jul 1, 2024

Thanks @skirpichev, I merged your PR.

I checked remaining buildbot failures: they are unrelated to the change.

@skirpichev: @erlend-aasland has concerns about relative #include.

@skirpichev skirpichev deleted the ctypes-c_complex-61103 branch July 1, 2024 09:56
@skirpichev
Copy link
Member Author

Thanks for reviews and patience.

@erlend-aasland has concerns about relative #include.

Yep, here; I'll open an issue (I think that build failures in this pr already don't need this;)). There are other includes, besides _math.h.

Akasurde pushed a commit to Akasurde/cpython that referenced this pull request Jul 3, 2024
…hon#120894)

Example:

```pycon
>>> import ctypes
>>> ctypes.__STDC_IEC_559_COMPLEX__
1
>>> libm = ctypes.CDLL('libm.so.6')
>>> libm.clog.argtypes = [ctypes.c_double_complex]
>>> libm.clog.restype = ctypes.c_double_complex
>>> libm.clog(1+1j)
(0.34657359027997264+0.7853981633974483j)
```

Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
…hon#120894)

Example:

```pycon
>>> import ctypes
>>> ctypes.__STDC_IEC_559_COMPLEX__
1
>>> libm = ctypes.CDLL('libm.so.6')
>>> libm.clog.argtypes = [ctypes.c_double_complex]
>>> libm.clog.restype = ctypes.c_double_complex
>>> libm.clog(1+1j)
(0.34657359027997264+0.7853981633974483j)
```

Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
…hon#120894)

Example:

```pycon
>>> import ctypes
>>> ctypes.__STDC_IEC_559_COMPLEX__
1
>>> libm = ctypes.CDLL('libm.so.6')
>>> libm.clog.argtypes = [ctypes.c_double_complex]
>>> libm.clog.restype = ctypes.c_double_complex
>>> libm.clog(1+1j)
(0.34657359027997264+0.7853981633974483j)
```

Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Victor Stinner <vstinner@python.org>
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.

6 participants