Repository navigation
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
With this PR I am not sure this is something that needs to be fixed. @skirpichev |
Maybe there is, see issue thread. But wrt current changeset - I would prefer rather to keep existing behavior. |
skirpichev
left a comment
There was a problem hiding this comment.
I think this will introduce speed regression on handling float subclasses with no override for __float__() dunder. Can you benchmark that?
|
You were right about the original patch: calling I tested a narrower fast path locally (keep
Normal If you're open to restoring the pre-3.14 |
|
I don't think that this pr needs any changes. Issue must be discussed first. |
Summary
The single-positional-argument form of
complex()has bypassed an overridden__float__on float subclasses since 3.14, returning the stored float value and suppressing exceptions raised by the override. Convert throughPyNumber_Float(), as the keyword and two-argument forms already do.Add regression coverage for the converted value and exception propagation, including construction of a complex subclass, and a NEWS entry.
Fixes #161926.
Testing
Built and tested on Windows x64 in Debug mode:
2c2d8018a20da33c749f42cc6a4ae55664a88321, the two new regression tests produce four failures: the overridden value is ignored and the expected exception is absent for both constructor classes.test_complex,test_capi.test_complex,test_cmath,test_float, andtest_builtinpass, with 306 tests run and 9 skipped.test_complexandtest_capi.test_complexalso pass-R 3:3reference-leak checks. Patchcheck, applicable pre-commit hooks, andgit diff --checkpass.The default full suite (
PCbuild\amd64\python_d.exe -m test -j 4 --timeout 180 --slowest) ran 52,079 tests with two failures, both also reproduced on an unmodified build of the same base commit.test_device_encodingdepends on the console handle and passes when the wholetest_os.test_osfile is rerun in a terminal.test_wrong_cert_tls13can receive a localized Windows connection-reset message that does not match its English-only assertion; the completetest_sslfile has the same failure on the base build.complex(x)ignores an overridden__float__of afloatsubclass since 3.14 #161926