Probably, from_float/decimal methods should be adapted to use this. Decimal.as_integer_ratio is in lowest terms. I guess, that same holds for the float method, but that's undocumented, unfortunately.

@skirpichev

I guess, that same holds for the float method, but that's undocumented, unfortunately

It does indeed. It would be reasonable to document it, I think.

@mdickinson,_normalize part was reverted. Does it make sense now?

In this way we can end with supporting everything as part of the API.

Indeed. :-) As with most aspects of software development, careful judgement is required, evaluating the context and balancing the trade offs within that context. Few things are black and white. The fact that_normalize was intended to be private, and has a leading underscore, certainly means that removing it is easier than if it were public, but it's still something that shouldn't be done without thinking it through first. Hyrum's law is a real thing.

@skirpichev

Does it make sense now?

Hmm, not really. :-) I don't want to leave a_normalize flag that's unused and untested, indefinitely. I think we have three options:

1. Leave this alone.
2. Introduce_from_pair (possibly after renaming - see below) and immediately remove the_normalize flag
3. Introduce_from_pair (ditto) and deprecate the_normalize flag.

Of those options, option 3 just seems like too much code churn for too little benefit.

There was a recentdiscuss.python.org thread in which the meaning of_normalize was misinterpreted, which pushes me a bit further towards option 2. Arguably,_normalize is too discoverable right now (since it shows up in IDE completions and tooltips), and there's a temptation to guess what it means._from_pair would also be somewhat discoverable, but at least we get to add a docstring that clearly indicates that it's private, and the expected constraints for people who decide to use it anyway.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Arguably, _normalize is too discoverable right now (since it shows up in IDE completions and tooltips)

That is a more generic problem: we have PEP8's convention where _single_leading_underscore is weak “internal use” indicator. On another hand, these arguments are available for introspection and shown in the help() output, IDE tooltips and so on.

Probably, all above use the signature() function. Maybe it does make sense to add aprivate=False option for this function to hide private kwargs per default?

There was a recent discuss.python.org thread in which the meaning of _normalize was misinterpreted

(Probably it was aboutthis thread.) Indeed, there was an impression that all methods do support unnormalized fractions.

PS:
I have made the requested changes; please review again.

Thanks for making the requested changes!

@mdickinson: please review the changes made to this pull request.

@skirpichev@mdickinson The changes look good to me. With the removal of the (semi-private) argument_normalize, we could update the whatsnew entry with a note stating that it is removed.

Thanks to all reviewers.