Repository: sympy/sympy

Issue:
`factorial2` runs into `RunTimeError` for non-integer
```
>>> from sympy import factorial2, S
>>> factorial2(S(5)/2)
File "/home/gxyd/Public/sympy/sympy/core/function.py", line 200, in __new__
    evaluated = cls.eval(*args)
  File "/home/gxyd/Public/sympy/sympy/functions/combinatorial/factorials.py", line 348, in eval
    return arg * (S.NegativeOne) ** ((1 - arg) / 2) / factorial2(-arg)
  File "/home/gxyd/Public/sympy/sympy/core/cache.py", line 93, in wrapper
    retval = cfunc(*args, **kwargs)
  File "/usr/lib/python3.4/functools.py", line 472, in wrapper
    result = user_function(*args, **kwds)
  File "/home/gxyd/Public/sympy/sympy/core/function.py", line 376, in __new__
    result = super(Function, cls).__new__(cls, *args, **options)
  File "/home/gxyd/Public/sympy/sympy/core/cache.py", line 93, in wrapper
    retval = cfunc(*args, **kwargs)
  File "/usr/lib/python3.4/functools.py", line 472, in wrapper
    result = user_function(*args, **kwds)
  File "/home/gxyd/Public/sympy/sympy/core/function.py", line 200, in __new__
    evaluated = cls.eval(*args)
  File "/home/gxyd/Public/sympy/sympy/functions/combinatorial/factorials.py", line 343, in eval
    return factorial(arg) / factorial2(arg - 1)
  File "/home/gxyd/Public/sympy/sympy/core/cache.py", line 93, in wrapper
    retval = cfunc(*args, **kwargs)
  File "/usr/lib/python3.4/functools.py", line 472, in wrapper
    result = user_function(*args, **kwds)
  File "/home/gxyd/Public/sympy/sympy/core/function.py", line 376, in __new__
    result = super(Function, cls).__new__(cls, *args, **options)
  File "/home/gxyd/Public/sympy/sympy/core/cache.py", line 93, in wrapper
    retval = cfunc(*args, **kwargs)
  File "/usr/lib/python3.4/functools.py", line 472, in wrapper
    result = user_function(*args, **kwds)
  File "/home/gxyd/Public/sympy/sympy/core/function.py", line 200, in __new__
    evaluated = cls.eval(*args)
  File "/home/gxyd/Public/sympy/sympy/functions/combinatorial/factorials.py", line 348, in eval
    return arg * (S.NegativeOne) ** ((1 - arg) / 2) / factorial2(-arg)
  File "/home/gxyd/Public/sympy/sympy/core/decorators.py", line 76, in __sympifyit_wrapper
    b = sympify(b, strict=True)
  File "/home/gxyd/Public/sympy/sympy/core/sympify.py", line 253, in sympify
    return converter[cls](a)
  File "/home/gxyd/Public/sympy/sympy/core/numbers.py", line 1666, in __new__
    if isinstance(i, string_types):
RuntimeError: maximum recursion depth exceeded while calling a Python object
```

Discussion:
jksuom: I looks like you have based the PR on a fairly old version of SymPy. It may be best to pull an up-to-date version first and then start afresh. As to the values of factorial2 for negative integers, not all of them need be infinite. It is possible to define finite values for negative odd arguments by a recurrence relation or rewrite formula.

Patch:
diff --git a/sympy/functions/combinatorial/factorials.py b/sympy/functions/combinatorial/factorials.py
index ea9e5a5ec0..48ea175555 100644
--- a/sympy/functions/combinatorial/factorials.py
+++ b/sympy/functions/combinatorial/factorials.py
@@ -330,8 +330,8 @@ def eval(cls, arg):
         # TODO: extend this to complex numbers?
 
         if arg.is_Number:
-            if arg.is_infinite:
-                return
+            if not arg.is_Integer:
+                raise ValueError("argument must be nonnegative integer or negative odd integer")
 
             # This implementation is faster than the recursive one
             # It also avoids "maximum recursion depth exceeded" runtime error
@@ -339,13 +339,13 @@ def eval(cls, arg):
                 if arg.is_even:
                     k = arg / 2
                     return 2 ** k * factorial(k)
-
                 return factorial(arg) / factorial2(arg - 1)
 
-            if arg.is_even:
-                raise ValueError("argument must be nonnegative or odd")
 
-            return arg * (S.NegativeOne) ** ((1 - arg) / 2) / factorial2(-arg)
+            if arg.is_odd:
+                return arg * (S.NegativeOne) ** ((1 - arg) / 2) / factorial2(-arg)
+            raise ValueError("argument must be nonnegative integer or negative odd integer")
+
 
     def _eval_is_even(self):
         # Double factorial is even for every positive even input


Test patch:
diff --git a/sympy/functions/combinatorial/tests/test_comb_factorials.py b/sympy/functions/combinatorial/tests/test_comb_factorials.py
index 9b54ea1019..1886a053ba 100644
--- a/sympy/functions/combinatorial/tests/test_comb_factorials.py
+++ b/sympy/functions/combinatorial/tests/test_comb_factorials.py
@@ -3,8 +3,9 @@
                    oo, zoo, simplify, expand_func, Product, I, Piecewise, Mod, Eq, sqrt)
 from sympy.functions.combinatorial.factorials import subfactorial
 from sympy.functions.special.gamma_functions import uppergamma
-from sympy.utilities.pytest import XFAIL
+from sympy.utilities.pytest import XFAIL, raises
 
+#Solves and Fixes Issue #10388 - This is the updated test for the same solved issue
 
 def test_rf_eval_apply():
     x, y = symbols('x,y')
@@ -194,7 +195,9 @@ def test_factorial2():
     nt = Symbol('nt', nonnegative=True)
     nf = Symbol('nf', nonnegative=False)
     nn = Symbol('nn')
-
+    #Solves and Fixes Issue #10388 - This is the updated test for the same solved issue
+    raises (ValueError, lambda: factorial2(oo))
+    raises (ValueError, lambda: factorial2(S(5)/2))
     assert factorial2(n).is_integer is None
     assert factorial2(tt - 1).is_integer
     assert factorial2(tte - 1).is_integer