Import abs in functions/all.py

[paulmasson]

Since abs is an explicit alias of abs_symbolic it should be explicitly imported from functions/other.py

Component: symbolics

Author: Paul Masson

Branch/Commit: u/paulmasson/import_abs_in_functions_all_py @ b36e1c4

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

[paulmasson]

Branch: u/paulmasson/import_abs_in_functions_all_py

[paulmasson]

Commit: b36e1c4

[paulmasson]

Author: Paul Masson

[paulmasson]

New commits:

b36e1c4

Import abs

[rwst]

comment:3

Patchbot:

sage -t --long src/sage/matrix/matrix2.pyx  # 5 doctests failed
sage -t --long src/sage/symbolic/expression.pyx  # 1 doctest failed
sage -t --long src/sage/rings/continued_fraction.py  # 2 doctests failed
sage -t --long src/sage/rings/number_field/number_field_element_quadratic.pyx  # 1 doctest failed
sage -t --long src/sage/modules/free_module_element.pyx  # 2 doctests failed
sage -t --long src/sage/stats/distributions/discrete_gaussian_lattice.py  # 2 doctests failed
sage -t --long src/sage/matrix/matrix_symbolic_dense.pyx  # 2 doctests failed
sage -t --long src/sage/matrix/matrix_double_dense.pyx  # 2 doctests failed
sage -t --long src/sage/misc/sage_input.py  # 1 doctest failed
sage -t --long src/sage/libs/mpmath/ext_main.pyx  # 1 doctest failed

[rwst]

comment:4

Commenting on some of the doctest failures...

sage -t --long --warn-long 35.3 src/sage/symbolic/expression.pyx
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 6868, in sage.symbolic.expression.Expression.__abs__
Failed example:
    abs(SR(-5),hold=True)
Expected:
    Traceback (most recent call last):
    ...
    TypeError: abs() takes no keyword arguments
Got:
    abs(-5)

This looks like the correct answer so just adapt the doctest.

File "src/sage/rings/continued_fraction.py", line 500, in sage.rings.continued_fraction.ContinuedFraction_base.__abs__
Failed example:
    abs(a)
Exception raised:
...
    TypeError: cannot coerce arguments: no canonical coercion from <class 'sage.rings.continued_fraction.ContinuedFraction_periodic'> to Symbolic Ring

Most fails are of this type. I think this is related to #17790.

sage -t --long src/sage/libs/mpmath/ext_main.pyx
...
      File "sage/symbolic/function.pyx", line 766, in sage.symbolic.function.Function._eval_mpmath_ (build/cythonized/sage/symbolic/function.cpp:8582)
        res = self(*args)
      File "sage/symbolic/function.pyx", line 979, in sage.symbolic.function.BuiltinFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:11061)
        return custom(*args)
      File "sage/symbolic/function.pyx", line 766, in sage.symbolic.function.Function._eval_mpmath_ (build/cythonized/sage/symbolic/function.cpp:8582)
        res = self(*args)
      File "sage/symbolic/function.pyx", line 979, in sage.symbolic.function.BuiltinFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:11061)
        return custom(*args)
...
    RuntimeError: maximum recursion depth exceeded while calling a Python object

This one is different, it looks like a bug in BuiltinFunction._eval_mpmath_().

[nbruin]

comment:5

Can we not merge this ticket and close it as invalid?

I think the alias should be interpreted the other way around: sage.functions.other.abs is bound because there's no name clash and this is the most logical name. In addition, there's sage.functions.other.abs_symbolic which avoids the nameclash with python's builtin abs and is therefore suitable for importing into the global namespace (does it need to be there, though?).

[rwst]

comment:6

My sage-devel post repeated here:
Expression.__abs__() directly calls Pynac's abs_eval while abs_symbolic goes through GinacFunction's Python code first, e.g.:

sage: abs_symbolic(x,x)
...
TypeError: Symbolic function abs takes exactly 1 arguments (2 given)

sage: abs(x,x)
...
TypeError: abs() takes exactly one argument (2 given)

so there is a difference that might show with other usages too.

[nbruin]

comment:7

Replying to @rwst:

My sage-devel post repeated here:
Expression.__abs__() directly calls Pynac's abs_eval while abs_symbolic goes through GinacFunction's Python code first

You can change that if you want to, but I'm not sure if one or the other has an advantage.

e.g.:

sage: abs_symbolic(x,x)
...
TypeError: Symbolic function abs takes exactly 1 arguments (2 given)

sage: abs(x,x)
...
TypeError: abs() takes exactly one argument (2 given)

That doesn't confirm the statement you made above. Python's abs does a check on the number of parameters before dispatching. It has to because it dispatches through a slot in PyNumberMethods, going to a c-function that accepts only one argument (meaning that for instance on cython types, abs(c) can work just as fast as len(c) etc: these methods are resolved via a pointer on the type; no method resolution required)

So the error traceback of abs(x,x) and abs_symbolic(x,x) will be different even if you chance Expression.__abs__.

so there is a difference that might show with other usages too.

These difference have a good reason, though, and are evidence of an efficiency in builtin.abs that we do not want to lose (e.g., people doing numerical work are going to be quite upset if they don't get the usual abs in sage anymore and have to pay the price of full method resolution and a GinacFunction).

Differences between toplevel functions and symbolic functions happen more often, even for things that are in control of sage:

sage: F=integrate(function('f')(x),x,0,1); F
integrate(f(x), x, 0, 1)
sage: integrate is F.operator()
False
sage: type(integrate)
<type 'function'>
sage: type(F.operator())
<class 'sage.symbolic.integration.integral.DefiniteIntegral'>
sage: integrate is sage.symbolic.integration.integral.integrate
False

[paulmasson]

comment:8

Replying to @nbruin:

Can we not merge this ticket and close it as invalid?

I really didn't expect this simple change to cause so much trouble, so yes let's close it. Thanks for the feedback.

[nbruin]

comment:10

Replying to @paulmasson:

I really didn't expect this simple change to cause so much trouble, so yes let's close it. Thanks for the feedback.

:-) Hopefully you enjoyed learning something about python and sage in the process.