Request for review: "gh-109598: make PyComplex_RealAsDouble/ImagAsDouble use __complex__"

This extends Make PyComplex_AsCComplex use __complex__ · Issue #44670 · python/cpython · GitHub to other PyComplex_As*() functions and fixes PyComplex_ImagAsDouble() that just silently returns 0.0 for all non-PyComplexObject objects.

PR: gh-109598: make PyComplex_RealAsDouble/ImagAsDouble use __complex__ by skirpichev · Pull Request #109647 · python/cpython · GitHub

Asking for review per suggestion in the devguide.

We might need to reevaluate our definitions of “concrete” and “abstract” API, as we’re turning more and more concrete APIs into abstract ones over time.

I’m not necessarily saying that’s a bad thing, but I do think it should be a deliberate thing.

(Possibly related discussion: Define guideline for parameter types: PyObject vs specific type · Issue #29 · capi-workgroup/api-evolution · GitHub)

cc: C-API working group @encukou @vstinner @iritkatriel @guido

1 Like

I am personally +1 on this. It’s weird that for floats it would call the dunder but the only complex number type it understands is PyComplexObject. I don’t know if it would break anything though, maybe we can try how the fix affects e.g. the numpy test suite? (If numpy doesn’t build with the main branch, a hack to test this might be to apply the patch to the 3.12 branch, just to see if anything breaks.)

Maybe if some code rely on a weird behaviour of PyComplex_ImagAsDouble() (silently return 0.0 on anything non-complex). New code accept more input.

It seems numpy uses this either in case of PyComplex_Type subclasses or when the argument has could be casted to PyFloat_Type with PyFloat_AsDouble(). That shouldn’t be broken.

BTW the numpy test suite locally - pass. (I run one on patched CPython 3.12.)

Maybe I miss some example (here is a quick search over Github), but it seems other consumers of this API (including the CPython itself) do same: i.e. check if the object is a PyComplex_Type (or subclass of) and then run some function.

Then I think there’s no objection to this PR from the “C API WG”. It definitely is weird that the current contract is “argument must be a PyComplexObject* or anything supporting __float__.” It makes more sense for it to be “argument must support __complex__ or __float__.”

We still need someone to review the PR though. :slight_smile:

2 Likes