Adapt to float factorial deprecation in Python 3.9

[slel]

Python 3.9 deprecates using math.factorial with floats with integral values.

• ​https://docs.python.org/3.9/library/math.html#math.factorial

  "Deprecated since version 3.9: Accepting floats with integral values (like 5.0) is deprecated."

This gives a doctest failure in Sage 9.2.rc0 with Python 3.9.0:

File "src/sage/functions/other.py", line 1629, in sage.functions.other.Function_factorial._eval_
Failed example:
    factorial(float(3.2))        # abs tol 1e-14
      File "<doctest sage.functions.other.Function_factorial._eval_[8]>", line 1, in <module>
        factorial(float(RealNumber('3.2')))        # abs tol 1e-14
      File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-py39/local/lib/python3.9/warnings.py", line 109, in _showwarnmsg
        sw(msg.message, msg.category, msg.filename, msg.lineno,
    :
    DeprecationWarning: Using factorial() with floats is deprecated
    7.756689535793181

First discussed in #30184 (comments 25, 33, 40, 41, 42, 43).

Upstream (pynac) report:

• Adapt to float factorial deprecation in Python 3.9 pynac/pynac#364

Upstream: Reported upstream. No feedback yet.

CC: @jhpalmieri @mkoeppe @slel @tscrim

Component: basic arithmetic

Keywords: factorial

Author: Frédéric Chapoton

Branch/Commit: e206151

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/30764

[jhpalmieri]

comment:2

Is this a pynac issue? I've been trying to understand how factorials work in Sage, and it looks like in the case of floats, it just passes it on to pynac. That is, factorial(float(3.2)) calls the __call__ method for Function in symbolic/function.pyx, and in this case it executes this code:

        if not symbolic_input and is_a_numeric(res):
            return py_object_from_numeric(res)

py_object_from_numeric comes from libs/pynac/pynac.pxd.

[slel]

comment:3

I opened a pynac ticket:

• Adapt to float factorial deprecation in Python 3.9 pynac/pynac#364

[slel]

Upstream: Reported upstream. No feedback yet.

[fchapoton]

comment:4

maybe the relevant code is here

https://github.com/pynac/pynac/blob/master/ginac/numeric.cpp#L3995

[fchapoton]

comment:5

Or maybe this goes through the __call__ method of the factorial function ?

sage: z=float(5.5)                                                              
sage: from sage.functions.other import Function_factorial                       
sage: myf = Function_factorial()                                                
sage: myf.__call__(z)                                                           
<ipython-input-84-e302b8a7557e>:1: DeprecationWarning: Using factorial() with floats is deprecated
  myf.__call__(z)
287.8852778150444
sage: myf._eval_(z)                                                             
287.8852778150444

whose doc says "Python float, Python complex, mpmath mpf and mpc as well as numpy inputs
are sent to the relevant math, cmath, mpmath or numpy
function"

[fchapoton]

comment:6

In python3.9, the code used depends on whether the float is integer-like or not:

For float(5.0), the __call__ method uses math.factorial that warns but works.

For float(5.1), the __call__ method uses math.factorial that raises an Error.
This error is catched, but what happens next is not very clear. Apparently, it uses another __call_, namely

                res = super(BuiltinFunction, self).__call__(
                        *args, coerce=coerce, hold=hold)

This then turns again to the module math, but for gamma, which works.

[fchapoton]

Commit: c614f96

[fchapoton]

Branch: u/chapoton/30764

[fchapoton]

Author: Frédéric Chapoton

[fchapoton]

comment:7

Here is a simple proposal, which works. If somebody has a much better idea, please provide an alternative proposal.

---

New commits:

c614f96

factorial of float => use gamma

[fchapoton]

comment:8

green bots (one with python 3.7, one with python 3.9), please review

[tscrim]

comment:10

This will work, but I think it would be better to have a little more detailed comment as the current one is lacking context. Perhaps something like:

# We do not include the factorial here as it is deprecated in Python 3.9.
# This case will be delegated to the gamma function.

[sagetrac-git]

Changed commit from c614f96 to e206151

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

8dc65e7	Merge branch 'u/chapoton/30764' in 9.3.b4
e206151	adding more explanations as comments

[fchapoton]

comment:12

ok, I have added (quite verbose) explanations + ref to this ticket

[tscrim]

comment:13

Thank you.

[tscrim]

Reviewer: Travis Scrimshaw

[vbraun]

Changed branch from u/chapoton/30764 to e206151