This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author wolma
Recipients akira, brg@gladman.plus.com, gladman, mark.dickinson, scoder, steven.daprano, vstinner, wolma
Date 2014-09-24.16:24:14
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1411575855.15.0.535929588438.issue22477@psf.upfronthosting.co.za>
In-reply-to
Content
Wouldn't it make more sense to change gcd() in the fractions module to return only positive integers?

The current gcd could become _gcd for private use by fractions, and the new wrapper gcd could just be implemented as:

def gcd(a,b):
    return abs(_gcd(a, b))


An aspect that hasn't really been discussed so far on the mailing list is that this is *not* only about whether the gcd of negative integers should be negative or positive, but also about the fact that in the current implementation the result of gcd is dependent on the order of the arguments:

>>> gcd(-3,6) == gcd(6,-3)
False

which I think is clearly unexpected.


@Steven:
I share your fear of generating backwards incompatibility only partially. Of course, there may be code out there that is relying on the current documented behavior for negative integer input, but probably not in too many places since after all it's a pretty special need and it's easy enough to implement that most people will not even come across the stdlib function before writing their own. Even if your code's affected it is really easily fixed.

I do admit though that the proposed change of behavior *could* cause hard-to-track bugs in pieces of code that *do* use fractions.gcd right now. So I do favor your scheme of raising a warning with negative numbers in Python 3.5, then changing the behavior in 3.6.

@Steven again:
I was playing around with the idea of having a gcd that accepts more than two arguments (and the same for a potential lcm function), then realized that its easily written right now as:

>>>reduce(gcd, (3,6,9))
3

Ironically, though, the proposed new gcd would make this slower than it has to be since it would do the abs() repeatedly, when

abs(reduce(_gcd, (3,6,9))) would suffice.

So, I guess that's the tradeoff coming with the proposed change:

- a more concise implementation of gcd

against

- broken backwards compatibility and
- reduced flexibility by calling abs implicitly instead of just when the user needs it

I guess with that I am 
+0.5 for the change though maybe an extra sentence in the docs about the consequences of the already documented behavior of gcd and that you really *should* call abs() on the result in most situations may be enough.
History
Date User Action Args
2014-09-24 16:24:15wolmasetrecipients: + wolma, mark.dickinson, scoder, vstinner, steven.daprano, akira, gladman, brg@gladman.plus.com
2014-09-24 16:24:15wolmasetmessageid: <1411575855.15.0.535929588438.issue22477@psf.upfronthosting.co.za>
2014-09-24 16:24:15wolmalinkissue22477 messages
2014-09-24 16:24:14wolmacreate