Skip to content
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

adjusting overload of fractions.Fraction.__new__ #13241

Merged
merged 2 commits into from
Dec 14, 2024

Conversation

tungol
Copy link
Contributor

@tungol tungol commented Dec 13, 2024

The existing overload has two issues, from stubtest's point of view. One is mixing up pos-only and kw-or-pos parameters between different branches of the overload (My MR python/mypy#18287 addresses that issue) and the other is the name of the parameters changing between overloads. (My MR would not fix that. stubtest currently tracks each parameter either by name or by position if it thinks it's pos-only. I think that handling this properly would require stubtest to track all parameters by both name and position, and make a decision about which one is relevant later in the process of signature generation, a much more complicated proposition.)

At any rate, "value" here is a fictional parameter name: it does not appear in the implementation anywhere. I looked in the history, and "value" appeared in the original version of fractions.pyi. I didn't see any particular discussion about it at any point.

I assume the idea was to add clarity; the documentation does a similar thing with parameter names of this class. I'm not familiar with the extended world of tools that consume typeshed, so this MR is less of a "we should definitely do this" on my part, and more of a "Let's have a discussion about it". This version is accurate to runtime. Does someone know how much this might affect the kinds of tooling that take typeshed as a source of documentation?

I think that "numerator" is technically correct even in the single argument form, if we assume a denominator of 1 in those cases. A fraction of (for example) 0.5/1 is valid, and "normalize the fraction 0.5/1" is an equivalent operation to "give me the fraction that corresponds to the value 0.5". So while value has more clarity, I don't think that numerator is outright wrong. That might not help much though.

This comment has been minimized.

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Thanks, I think this makes the stub more accurate. If "numerator" is inaccurate, it should be fixed in CPython, but as you say it's at least not terribly wrong.

Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JelleZijlstra JelleZijlstra merged commit acebd50 into python:main Dec 14, 2024
63 checks passed
@tungol tungol deleted the fractions branch December 14, 2024 21:39
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.

2 participants