GenericGraph.[weighted]adjacency_matrix, incidence_matrix: Accept keyword arguments for matrix constructor

[mkoeppe]

Depends on #32465

CC: @dcoudert

Component: graph theory

Author: Matthias Koeppe

Branch/Commit: 0bd2930

Reviewer: David Coudert

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

[mkoeppe]

Branch: u/mkoeppe/use_numpy_integer_dense_for_adjacency_matrices

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Commit: 01d5541

[mkoeppe]

Last 10 new commits:

2ffb118	Vector_double_dense: Factor through new class Vector_numpy_dense
5b288c3	sage.modules.vector_numpy_integer_dense: New
c8eec7e	Merge tag '9.5.beta6' into t/32465/refactor_matrix_double_dense_through_matrix_numpy_dense
5323b25	src/sage/matrix/matrix_numpy_dense.pyx: Returns -> Return
d07cf93	Merge tag '9.5' into t/32465/refactor_matrix_double_dense_through_matrix_numpy_dense
93f3925	src/sage/matrix/matrix_numpy_dense.pyx: Fix doc markup, whitespace
21ed44a	Merge tag '9.6.beta1' into t/32465/refactor_matrix_double_dense_through_matrix_numpy_dense
fe1da0a	Merge #32465
007253d	src/sage/matrix/matrix_space.py (get_matrix_class): Handle base_ring=ZZ, implementation='numpy'
01d5541	GenericGraph.adjacency_matrix: Accept keyword arguments for matrix constructor

[dcoudert]

comment:3

I'm OK with this improvement, but the same should be done for weighted_adjacency_matrix.

These methods could benefit further improvements. So far we first feed a dictionary and then build the matrix. It might be faster to directly feed the matrix.

[dcoudert]

Reviewer: David Coudert

[mkoeppe]

comment:4

Thanks for the feedback! Here's the corresponding change for weighted_adjacency_matrix.

[sagetrac-git]

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

03343f7	GenericGraph.adjacency_matrix: Add doctest with immutable=True
688d054	GenericGraph.weighted_adjacency_matrix: Accept keyword arguments for matrix constructor

[sagetrac-git]

Changed commit from 01d5541 to 688d054

[mkoeppe]

comment:6

Replying to @dcoudert:

These methods could benefit further improvements. So far we first feed a dictionary and then build the matrix. It might be faster to directly feed the matrix.

Yes, but I won't work on it in this ticket

[sagetrac-git]

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

e30c7cb

GenericGraph.weighted_adjacency_matrix: Make base_ring a keyword-only argument

[sagetrac-git]

Changed commit from 688d054 to e30c7cb

[sagetrac-git]

Changed commit from e30c7cb to 6f2185a

[sagetrac-git]

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

6f2185a

GenericGraph.incidence_matrix: Accept keyword arguments for matrix constructor

[sagetrac-git]

Changed commit from 6f2185a to 0bd2930

[sagetrac-git]

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

0bd2930

GenericGraph._matrix_: Use the new keyword argument base_ring of GenericGraph.adjacency_matrix

[mkoeppe]

comment:10

I'll stop here, but the same could be done to BipartiteGraph.reduced_adjacency_matrix and some other methods that I see in git grep ' def.*_matrix' src/sage/graphs/

For distance_matrix, kirchhoff_matrix it is trickier because these methods already accept **kwds that they pass on to some other method.

[dcoudert]

comment:12

LGTM.

I will check the other cases.

[dcoudert]

comment:13

For kirchhoff_matrix, all arguments in **kwds are passed to adjacency_matrix or weighted_adjacency_matrix. So it is now possible to specify the base ring.

A question is whether we should allow passing arguments for the matrix constructor to methods calling adjacency_matrix and similar methods.

[mkoeppe]

comment:14

Replying to @dcoudert:

For kirchhoff_matrix, all arguments in **kwds are passed to adjacency_matrix or weighted_adjacency_matrix. So it is now possible to specify the base ring.

Thanks for checking! If immutable=True is passed, the result matrix will have to be set_immutable. Could be done in #33388

[mkoeppe]

comment:15

Thanks for reviewing!

[vbraun]

Changed branch from u/mkoeppe/use_numpy_integer_dense_for_adjacency_matrices to 0bd2930