Bad error message for weighted adjacency matrix

[kcrisman]

H=Graph({0:[1,2,3], 4:[0,2], 6:[1,2,3,4,5]})
H.weighted(True)
H.weighted_adjacency_matrix()

should give something useful, but instead is an inexplicable error message

TypeError: NoneType takes no arguments

This happens whether or not this graph is said to be weighted. There should be an error catch for a useful message.

Component: graph theory

Author: David Coudert

Branch/Commit: fe8b4d5

Reviewer: Dima Pasechnik

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

[dcoudert]

comment:1

It's not the only undesirable behavior of methods using weighted.

sage: G = Graph([(0, 1)])
sage: G.weighted_adjacency_matrix()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: NoneType takes no arguments
sage: G.weighted_adjacency_matrix(base_ring=ZZ)
[0 0]
[0 0]
sage: G = Graph([(0, 1, '12')])
sage: G.weighted_adjacency_matrix(base_ring=ZZ)
[ 0 12]
[12  0]

This said, I'm not sure which is the best solution here. Should we first check edge weights (with/without using a base ring) ? force to pass a weight function ? Something else ?

[dcoudert]

Commit: aaabe1d

[dcoudert]

Author: David Coudert

[dcoudert]

comment:3

I added a check of edge weights. Let me know if this is what you expect.

---

New commits:

aaabe1d

trac #33562: better error message

[dcoudert]

Branch: public/graphs/33562

[dcoudert]

comment:5

Green bot, please review.

[dimpase]

comment:6

Why can't 'a' be a weight?

IMHO whatever values we have on edges may be weights.

[dcoudert]

comment:7

'a' can be a label, but not a weight. So far we always consider weights as numbers that you can use for instance to compute a shortest path or a minimum weight spanning tree.

[dimpase]

comment:8

Replying to David Coudert:

'a' can be a label, but not a weight. So far we always consider weights as numbers that you can use for instance to compute a shortest path or a minimum weight spanning tree.

sure, but we're talking about the adjacency matrix with the edge labels put at their respective places. These labels may be e.g. polynomials, or complex numbers - objects on which there is no natural order to speak about. Why would one want to forbid such matrices? E.g. in linear algebra one naturally may represent sparse matrices as graphs.

[sagetrac-git]

Changed commit from aaabe1d to b6abee8

[sagetrac-git]

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

109c793	trac #33562: merge with 9.8.beta2
b6abee8	trac #33562: add paramter default_weight

[dcoudert]

comment:10

Here is an alternative solution that catches None and proposes to set a default_weight. This should answer the issue raised in this ticket and let users manipulate different types of labels.
Note however that 'a' is not accepted by the matrix constructor.

[sagetrac-git]

Changed commit from b6abee8 to 7dee543

[sagetrac-git]

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

f382241	trac #33562: better error message
7dee543	trac #33562: add paramter default_weight

[dimpase]

Reviewer: Dima Pasechnik

[dimpase]

comment:12

OK, rebased over the latest beta. Looks good.

By the way, edge weights are allowed to be ring elements, e.g. SR, or polynomial rings. So it's OK as far as I am concerned - but of course doing any kind of e.g. shortest path computation (anything that assumes that weights are comparable with <) on such graphs makes little sense.

[vbraun]

comment:13

Merge failure on top of:

52abf762eeb Trac #24462: Add tests that discriminant() of number fields is Integer

48afcb931df Trac #34713: Update IPython to 8.6

3c3b748f895 Trac #34712: partial pep8 cleanup for number_field.py

bdcd8e2f903 Trac #34707: Bug in Clifford algebra multiplication

ab40677de1c Trac #34702: fix deprecated use of PyEval_Call*

b8151dc1099 Trac #34681: Error with multiplication of points on elliptic curves over Integers(n)

0fcca83be81 Trac #34651: Add multivariate_interpolation for multivariate polynomial rings

7c37d8de415 Trac #34611: fast implementation of exp

0a63c62c2da Trac #34397: pycodestyle cleanup in src/sage/graphs/generic_graph.py (part 9)

1f9b781eee1 Trac #34081: Upgrade scipy to 1.9.x, add meson toolchain

58c9281 Trac #32267: make hadamard_matrix() use all the Hadamard matrices Sage knows

edfa1f9 Trac #30423: F-Matrix Factory

7aa226c Trac #34699: some details about INPUT and INPUT in the doc

c4079b7 Trac #34697: minor fixes in doc in pyx files

8130728 Trac #34691: companion matrix of constant polynomial has the wrong parent

6b70584 Trac #34638: refresh the file categories/rings.py

f634f6b Trac #34368: implement the F,H,M triangles

859c351 Trac #34689: make Compositions() an additive monoid

b508288 Trac #34665: openssl spkg-configure.m4: Also require openssl if curl needs to be built

487f2f9 Trac #34662: sage.combinat.permutation.from_cycles produces wrong result when 'cycles' is a generator

ab0944d Trac #34636: make sparsity a decision of the user

24d0a8a Trac #34381: Add infinite q-Pochhammer symbol

ef1d3d2 Trac #34260: Implement northwest diagrams

bff11ac Trac #33176: Fix a few cython "referenced before assignment" warnings

89af346 Trac #32267: make hadamard_matrix() use all the Hadamard matrices Sage knows

c3028e7 Updated SageMath version to 9.8.beta3

merge was not clean: conflicts in src/sage/graphs/generic_graph.py

[sagetrac-git]

Changed commit from 7dee543 to fe8b4d5

[sagetrac-git]

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

fe8b4d5

trac #33562: fix merge conflict with 9.8.beta4

[dcoudert]

comment:15

I fixed the merge conflict => back to positive review.

[vbraun]

Changed branch from public/graphs/33562 to fe8b4d5