Graph: accept keyword arguments for matrix constructor in methods returning a matrix

[dcoudert]

Following #33377, we allow methods returning a matrix in Graph to pass arguments to the matrix constructors.

We also fix 33377#comment:14.

Depends on #33377

CC: @mkoeppe

Component: graph theory

Author: David Coudert

Branch: bdf02bd

Reviewer: Matthias Koeppe

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

[dcoudert]

Branch: public/graphs/33388

[dcoudert]

Last 10 new commits:

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
03343f7	GenericGraph.adjacency_matrix: Add doctest with immutable=True
688d054	GenericGraph.weighted_adjacency_matrix: Accept keyword arguments for matrix constructor
e30c7cb	GenericGraph.weighted_adjacency_matrix: Make base_ring a keyword-only argument
6f2185a	GenericGraph.incidence_matrix: Accept keyword arguments for matrix constructor
0bd2930	GenericGraph._matrix_: Use the new keyword argument base_ring of GenericGraph.adjacency_matrix
39da87c	trac #33388: merge with 9.6.beta2
7e5ae03	trac #33388: Graph: accept keyword arguments for matrix constructor in methods returning a matrix
90a05b2	trac #33388: check parameter immutable in kirchhoff_matrix

[dcoudert]

Commit: 90a05b2

[mkoeppe]

comment:2

+        if 'immutable' in kwds:
+            set_immutable = kwds['immutable']
+            del kwds['immutable']
+        else:
+            set_immutable = False

This can be replaced by set_immutable = kwds.pop('immutable', False)

[mkoeppe]

comment:3

Also M can be constructed as immutable in this function because the code is not mutating M.

[mkoeppe]

comment:4

Similar comment on seidel_adjacency_matrix. Perhaps you could construct C as mutable, A as immutable, and then do an in-place subtraction to construct a mutable or immutable result.

[mkoeppe]

comment:5

For effective_resistance, as the result is a scalar, I'm not sure if exposing all matrix keywords is a good idea. Perhaps only base_ring?

[mkoeppe]

comment:6

Replying to @mkoeppe:

Similar comment on seidel_adjacency_matrix. Perhaps you could construct C as mutable, A as immutable, and then do an in-place subtraction to construct a mutable or immutable result.

Likewise for effective_resistance_matrix

[mkoeppe]

comment:7

Also in common_neighbors_matrix, handling immutable would be good

[mkoeppe]

Reviewer: Matthias Koeppe

[sagetrac-git]

Changed commit from 90a05b2 to bdf02bd

[sagetrac-git]

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

bdf02bd

trac #33388: review comments

[dcoudert]

comment:10

I tried to address all your comments.

[mkoeppe]

comment:11

LGTM. Green patchbot => positive review

[mkoeppe]

comment:12

The patchbot failure is unrelated

[vbraun]

Changed branch from public/graphs/33388 to bdf02bd

[sagetrac-tmonteil]

Changed commit from bdf02bd to none

[sagetrac-tmonteil]

comment:14

Patchbots are locked because of some numerical noise in a doctest introduced in this ticket, follow-up : #33427