[issue32466] Remove fractions._gcd()

2018-01-01 Thread Mark Dickinson
Mark Dickinson added the comment: > the types in question were added to the numeric tower for gmpy 2.0.9 and > 2.1.0. Ah, good to know. Thanks. I was using 2.0.8. > * We're back to needing a test for the line in question. Agreed. It's not currently covered, and with the current behaviour the

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Gordon P. Hemsley
Gordon P. Hemsley added the comment: So, if I'm understanding your position correctly: * We're back to needing a test for the line in question. * We're eschewing the possibility of changing the behavior of `fractions.Fraction` to force int numerator and denominator. Is that correct?

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Gordon P. Hemsley
Gordon P. Hemsley added the comment: Side note: https://github.com/aleaxit/gmpy/issues/127 suggests that the types in question were added to the numeric tower for gmpy 2.0.9 and 2.1.0. -- ___ Python tracker

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Mark Dickinson
Mark Dickinson added the comment: > but it would result in a behaviour change: for the above code, we'd get a > `Fraction` whose numerator and denominator were both of actual type `int` > instead of `mpz` Ah, sorry. That's not true in this particular case. The returned gcd would be of type `

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Mark Dickinson
Mark Dickinson added the comment: > if math.gcd() only accepts integers and not, at least, numbers.Integral, > wouldn't that be a bug? I'd call it an enhancement opportunity rather than a bug. :-) There's no general Python-wide requirement that an instance of numbers.Integral should be accep

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Mark Dickinson
Mark Dickinson added the comment: Here's a concrete example, using gmpy2 [1], that exercises the code path you're proposing to remove. I had to cheat a bit, since the gmpy2 types *don't* (currently) buy in to the numbers ABC tower (though there's no good reason that they couldn't do so), so I

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Mark Dickinson
Mark Dickinson added the comment: > In what scenario would the numerator and denominator be numbers.Rational but > not an integer or a fraction But that's not the issue here. The issue here is having an instance of `numbers.Rational` whose numerator and denominator are not specifically of co

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Gordon P. Hemsley
Gordon P. Hemsley added the comment: Indeed, that is the code fragment I was referring to. Mathematically speaking, a rational number is one that can be expressed as a fraction of two integers, so in that regard the numerator and the denominator should both be integers. But let's assume for

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Mark Dickinson
Mark Dickinson added the comment: > [...] I cannot envision a scenario where that would be possible [...] I don't think it can be ruled out. If I'm reading the code right, it's preceded by this branch of the initial if/elif chain: elif (isinstance(numerator, numbers.Rational) and

[issue32466] Remove fractions._gcd()

2017-12-31 Thread Gordon P. Hemsley
New submission from Gordon P. Hemsley : I noticed that there was a single line of Lib/fractions.py that did not have test coverage: the normalize step for fractions with non-integer numerators and/or denominators. I initially was going to implement a test for that line, but upon further refle