abs(matrix) should not be a shortcut for det

[videlec]

We have currently

sage: M = matrix([[-1]])
sage: abs(M)
-1

Because matrix.__abs__ is a shortcut for determinant!!I

n scipy, __abs__ applies the absolute value to each coefficient. But it is not likely what we want to do in Sage. Instead we raise a TypeError and inform the user about matrix.norm(1) and matrix.apply_map(abs).

Related discussion on sage-devel https://groups.google.com/forum/#!topic/sage-devel/pFI9y3YZIQQ

Component: linear algebra

Author: Vincent Delecroix

Branch/Commit: e6a4de8

Reviewer: Nathann Cohen

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

[videlec]

Commit: 697c759

[videlec]

New commits:

697c759

trac #17443: fix abs(matrix)

[videlec]

Branch: u/vdelecroix/17443

[nathanncohen]

comment:2

Crazy O_o

I will run the tests and see if it breaks anything.

Why do you make "abs" an alias for __abs__ ? If you believe that it is worth adding a function (given that abs(M) works I do not understand why it would be), can you make it .absolute_value ? We try to not have full names for methods.

Nathann

[videlec]

comment:3

No hurry. Discussion going on on sage-devel...

[jdemeyer]

comment:4

The discussions seem to indicate a preference for deprecating __abs__.

[nathanncohen]

comment:5

The discussions seem to indicate a preference for deprecating __abs__.

Yesyes, this seems to be the wisest thing to do indeed.

Nathann

[sagetrac-git]

Changed commit from 697c759 to f4ac40f

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

f4ac40f

trac #17443: deprecate abs + generic apply_map

[jdemeyer]

comment:8

This should be a deprecation, not an error.

[sagetrac-git]

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

e6a4de8

trac #17443: change TypeError to a deprecation

[sagetrac-git]

Changed commit from f4ac40f to e6a4de8

[videlec]

comment:10

All right, it is now a deprecation...

[nathanncohen]

comment:11

Passes all tests, so positive review.

Do you know of a tool to detect that you only "moved" the code of those two big functions ?

I found no way to do this, so in order to check your patch I moved what you wrote in matrix2 back to matrix_dense, only to see as it was detected as "leaving those function as they were" when merged with your patch.

Nathann

[nathanncohen]

Reviewer: Nathann Cohen

[vbraun]

Changed branch from u/vdelecroix/17443 to e6a4de8