[issue33083] math.factorial accepts non-integral Decimal instances

[Mark Dickinson]

Mark Dickinson <dickinsm at gmail.com> added the comment:

So I'm against _adding_ support for Decimal and Fraction on various grounds:

(1) If it's our intention to deprecate acceptance of non-integral types, then it seems odd to deliberately add a new intentional feature (acceptance of integral-valued Decimal objects) that we know we want to deprecate long term. It's only if we don't intend any such deprecation that it would make sense to add those new features.

(2) Implementation: adding support for Decimal objects in a math module is going to be messy, adding significant complication to the C code. The math module would have to import Decimal (something it doesn't currently need to do) in order to make the Decimal instance checks. @taleinat: how would you see the implementation working?

(3) If we support Fraction, should we also support arbitrary numbers.Rational instances? What's the exact rule for what should and shouldn't be accepted. The line isn't clear. In contrast, if the rule is that only floats and integer-like things are accepted, it's much clearer.

I think there's a clear goal here, which is that `math.factorial` should accept only integral types, defined as those implementing `__index__`. This is the same thing that `math.gcd` does.

To avoid gratuitous breakage, and because acceptance of floats was a deliberate design decision, we should continue to accept integral floats in the short term, perhaps eventually deprecating. But deliberately supporting integral Decimals and Fractions takes us further from the goal.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue33083>
_______________________________________