Skip to content

gh-94673: Ensure subtypes are readied only once in math.trunc() #105465

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jun 7, 2023
Merged

gh-94673: Ensure subtypes are readied only once in math.trunc() #105465

merged 1 commit into from
Jun 7, 2023

Conversation

neonene
Copy link
Contributor

@neonene neonene commented Jun 7, 2023

@neonene neonene changed the title gh-94673: Ensure subtypes are readied once in math.trunc() gh-94673: Ensure subtypes are readied only once in math.trunc() Jun 7, 2023
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

(good catch!)

@miss-islington
Copy link
Contributor

Thanks @neonene for the PR, and @ericsnowcurrently for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-105471 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 7, 2023
…pythongh-105465)

Fixes a typo in d2e2e53.
(cherry picked from commit 5394bf9)

Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.12 only security fixes label Jun 7, 2023
@neonene neonene deleted the math_trunc branch June 7, 2023 20:06
ericsnowcurrently pushed a commit that referenced this pull request Jun 7, 2023
gh-105465) (gh-105471)

Fixes a typo in d2e2e53.
(cherry picked from commit 5394bf9)

Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
@mdickinson
Copy link
Member

Out of curiosity, why is the code for math_trunc different from that for math_floor and math_ceil? Why does math_trunc need the PyType_Ready call, but not math_floor or math_ceil?

@ericsnowcurrently
Copy link
Member

Hmm, I have a guess. That code was added in 2007 (commit 15d3d04) as a builtin (bltinsmodule.c). In 2008 the function was moved to the math module. So that's probably the difference.

@mdickinson
Copy link
Member

@ericsnowcurrently Nice find; that would make sense. (Well, maybe; I still can't quite figure out why the PyType_Ready would have been necessary, but it's a little more plausible that built-in functions could be being executed before some types are fully initialised.)

Do you see any reason not to just get rid of this code in mathmodule.c?

@ericsnowcurrently
Copy link
Member

I suspect that code is an artifact of a time when folks were inconsistent about calling PyType_Ready(). However, I'm not sure what the actual status quo is at this point. This might be a good thing to ask about on python-dev.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants