Skip to content

gh-136169: Update parameter name in fractions.from_float method #136172

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 1 commit into from
Jul 1, 2025

Conversation

lazorikv
Copy link
Contributor

@lazorikv lazorikv commented Jul 1, 2025

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks!

@sobolevn sobolevn added awaiting core review needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes and removed awaiting merge labels Jul 1, 2025
@sobolevn sobolevn merged commit 9c0cb5b into python:main Jul 1, 2025
48 checks passed
@miss-islington-app
Copy link

Thanks @lazorikv for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

@github-project-automation github-project-automation bot moved this from Todo to Done in Docs PRs Jul 1, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 1, 2025
…pythonGH-136172)

Update parameter name in fractions.from_float method
(cherry picked from commit 9c0cb5b)

Co-authored-by: Vladyslav Lazoryk <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 1, 2025

GH-136174 is a backport of this pull request to the 3.14 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 1, 2025
…pythonGH-136172)

Update parameter name in fractions.from_float method
(cherry picked from commit 9c0cb5b)

Co-authored-by: Vladyslav Lazoryk <[email protected]>
@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Jul 1, 2025
@bedevere-app
Copy link

bedevere-app bot commented Jul 1, 2025

GH-136175 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 1, 2025
@ZeroIntensity
Copy link
Member

If anyone is interested in a follow-up, decimal has a similar method, but the issue there is that Decimal.from_float doesn't accept kwargs at all, so from_float(f=...) breaks.

sobolevn pushed a commit that referenced this pull request Jul 1, 2025
GH-136172) (#136175)

gh-136169: Update parameter name in fractions.from_float method (GH-136172)

Update parameter name in fractions.from_float method
(cherry picked from commit 9c0cb5b)

Co-authored-by: Vladyslav Lazoryk <[email protected]>
sobolevn pushed a commit that referenced this pull request Jul 1, 2025
GH-136172) (#136174)

gh-136169: Update parameter name in fractions.from_float method (GH-136172)

Update parameter name in fractions.from_float method
(cherry picked from commit 9c0cb5b)

Co-authored-by: Vladyslav Lazoryk <[email protected]>
@skirpichev
Copy link
Contributor

skirpichev commented Jul 1, 2025

but the issue there is that Decimal.from_float doesn't accept kwargs at all

C extension method - doesn't, but for pure-Python case - does:

>>> import inspect, _pydecimal, _decimal
>>> inspect.signature(_pydecimal.Decimal.from_float)
<Signature (f)>
>>> inspect.signature(_decimal.Decimal.from_float)
<Signature (f, /)>

As explained in the issue thread, the argument name is not a part of API. IMO, this pr makes docs a little worse.

from_float(f=...) breaks

Have you find some such code?

@ZeroIntensity
Copy link
Member

C extension method - doesn't, but for pure-Python case - does:

Ugh, yuck. I'm not sure then. It's probably safest to force the Python implementation to be positional-only.

@skirpichev
Copy link
Contributor

There is a long list of issues, related to pydecimal/decimal incompatibilities (mostly closed), e.g. #117056.

It's probably safest to force the Python implementation to be positional-only.

I'm not sure if it does make sense to maintain it. We have a mature C extension and, IMO, the decimal module can be optional.

@terryjreedy terryjreedy added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jul 1, 2025
@miss-islington-app

This comment was marked as duplicate.

@miss-islington-app

This comment was marked as duplicate.

@miss-islington-app

This comment was marked as duplicate.

@miss-islington-app

This comment was marked as duplicate.

@miss-islington-app miss-islington-app bot assigned sobolevn and unassigned terryjreedy Jul 1, 2025
@terryjreedy terryjreedy removed needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Jul 1, 2025
@StanFromIreland
Copy link
Member

@terryjreedy This has already been backported see: 3.14, 3.13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants