Make .coxeter_matrix() return a CoxeterMatrix for coxeter3-implemented groups

[cemulate]

This patch fixes all of the following, which currently throw errors:

W = CoxeterGroup(['B', 3], implementation='coxeter3')
W.coxeter_type()
# AttributeError: 'sage.matrix.matrix_integer_dense.Matrix_integer_dense' object has no attribute 'coxeter_type'
W([1,2,1]).reduced_words()
# IndexError: matrix index out of range
R.<v> = LaurentPolynomialRing(ZZ)
IwahoriHeckeAlgebra(W, v, -1/v)
# AttributeError: 'sage.matrix.matrix_integer_dense.Matrix_integer_dense' object has no attribute 'coxeter_type'

The underlying problem in all cases is that W.coxeter_matrix() does not return a CoxeterMatrix; I've altered this to return the (correctly indexed) coxeter matrix and added a test.

This also removes a completely unused function CoxeterGroup.m(i, j) which seemingly existed only to workaround the fact that .coxeter_matrix() was incorrectly indexed (and in fact is ill-founded, it would cause an error on a group with affine Cartan type). This removal breaks no tests in libs/coxeter3.

Component: combinatorics

Keywords: CoxeterGroup, CoxeterMatrix, coxeter, coxeter3

Author: Chase Meadors

Branch/Commit: bd9b5c6

Reviewer: Travis Scrimshaw

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

[cemulate]

Branch: u/gh-cemulate/coxeter3_matrix_cleanup

[sagetrac-git]

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

c1b4446

Add test

[sagetrac-git]

Commit: c1b4446

[cemulate]

New commits:

c1b4446

Add test

[cemulate]

Changed keywords from none to CoxeterGroup, CoxeterMatrix, coxeter, coxeter3

[cemulate]

Author: Chase Meadors

[tscrim]

Reviewer: Travis Scrimshaw

[tscrim]

comment:4

The m() method might not be used in the code, but some user might be using it in their personal code. You should issue a deprecation instead with the appropriate redirect:

from sage.misc.superseded import deprecation
deprecation(30237, "the .m(i, j) method has been deprecated; instead use .coxeter_matrix()[i,j]")

or whatever message is appropriate.

Other than that, LGTM.

[sagetrac-git]

Changed commit from c1b4446 to d5a7473

[sagetrac-git]

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

d5a7473

Add deprecation for removed method.

[cemulate]

comment:6

Done.

[tscrim]

comment:7

It needs to be doctested with a docstring as well.

[sagetrac-git]

Changed commit from d5a7473 to bc2e1d1

[sagetrac-git]

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

bc2e1d1

Add doctests for deprecated method.

[cemulate]

comment:9

Ah, sorry, wasn't sure if that needed to be done in this case. It's there now.

[tscrim]

comment:10

Can you quickly add a docstring just saying This is deprecated, use ``self.coxeter_matrix()[i,j]`` instead.?

[sagetrac-git]

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

86fd6ec

Add docstring for deprecated method.

[sagetrac-git]

Changed commit from bc2e1d1 to 86fd6ec

[cemulate]

comment:12

Done.

[sagetrac-git]

Changed commit from 86fd6ec to c55ec05

[sagetrac-git]

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

c55ec05

Mark other doctest line optional.

[tscrim]

comment:14

Sorry, one last thing. It needs to actually have the same result/behavior as before.

[sagetrac-git]

Changed commit from c55ec05 to bd9b5c6

[sagetrac-git]

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

bd9b5c6

Make deprecated method have same behavior.

[cemulate]

comment:16

I hope I understood correctly; I made it have the same behavior as before by "fixing" it; should it instead have the same implementation as before (which would cause the old doctests to throw errors)? Thanks for guiding me through the protocol here, by the way.

[tscrim]

comment:17

Perfect, thank you.

[vbraun]

Changed branch from u/gh-cemulate/coxeter3_matrix_cleanup to bd9b5c6