Opened 5 years ago

Closed 5 years ago

# (EASY) bug in constructing an error message for DiGraph constructor

Reported by: Owned by: was trivial sage-8.0 graph theory David Coudert Travis Scrimshaw N/A f4b45dc f4b45dc8654be7d0d194448a17dc6689ac7cc730

### Description

```~\$ sage/sage
┌────────────────────────────────────────────────────────────────────┐
│ SageMath version 8.0.beta5, Release Date: 2017-05-04               │
│ Type "notebook()" for the browser-based notebook interface.        │
│ Type "help()" for help.                                            │
└────────────────────────────────────────────────────────────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
┃ Warning: this is a prerelease version, and it may be unstable.     ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛
sage: DiGraph(matrix ([[1,0,0,1],[0,0,1,1],[0,0,1,1]]).transpose())
---------------------------------------------------------------------------
UnboundLocalError                         Traceback (most recent call last)
<ipython-input-1-8da17564bcf0> in <module>()
----> 1 DiGraph(matrix ([[Integer(1),Integer(0),Integer(0),Integer(1)],[Integer(0),Integer(0),Integer(1),Integer(1)],[Integer(0),Integer(0),Integ
er(1),Integer(1)]]).transpose())

/projects/68c8b2b8-03ba-44d4-a0d1-5d771c8cb465/sage/local/lib/python2.7/site-packages/sage/graphs/digraph.pyc in __init__(self, data, pos, loops,
format, weighted, implementation, data_structure, vertex_labels, name, multiedges, convert_empty_dict_labels_to_None, sparse, immutable)
689         elif format == 'incidence_matrix':
690             from .graph_input import from_oriented_incidence_matrix
--> 691             from_oriented_incidence_matrix(self, data, loops=loops, multiedges=multiedges, weighted=weighted)
692
693         elif format == 'DiGraph':

/projects/68c8b2b8-03ba-44d4-a0d1-5d771c8cb465/sage/local/lib/python2.7/site-packages/sage/graphs/graph_input.pyc in from_oriented_incidence_matr
ix(G, M, loops, multiedges, weighted)
381         L = sorted(set(c.list()))
382         if L != [-1,0,1]:
--> 383             msg += "Each column represents an edge: -1 goes to 1."
384             raise ValueError(msg)
385         if c[NZ[0]] == -1:

UnboundLocalError: local variable 'msg' referenced before assignment
```

Fix is to set msg = at the beginning...

### comment:1 Changed 5 years ago by dcoudert

actually, in the code of `from_oriented_incidence_matrix` in file `graph_input.py`, we have:

```            msg += "Each column represents an edge: -1 goes to 1."
raise ValueError(msg)
```

and nothing else is added to msg. It was may be the case before the init method of `DiGraph` was split into multiple methods since the first instruction of the init method is `msg = ''`. So it suffices to put the message inside the `ValueError`.

Are you doing the patch or should we do it ?

### comment:2 Changed 5 years ago by dcoudert

• Authors set to David Coudert
• Branch set to u/dcoudert/22985
• Commit set to f4b45dc8654be7d0d194448a17dc6689ac7cc730
• Status changed from new to needs_review

I pushed a proposal to fix the bug. Now it raises an error.

```sage: DiGraph(matrix ([[1,0,0,1],[0,0,1,1],[0,0,1,1]]).transpose())
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-1-8da17564bcf0> in <module>()
----> 1 DiGraph(matrix ([[Integer(1),Integer(0),Integer(0),Integer(1)],[Integer(0),Integer(0),Integer(1),Integer(1)],[Integer(0),Integer(0),Integer(1),Integer(1)]]).transpose())

/Users/dcoudert/sage/local/lib/python2.7/site-packages/sage/graphs/digraph.pyc in __init__(self, data, pos, loops, format, weighted, implementation, data_structure, vertex_labels, name, multiedges, convert_empty_dict_labels_to_None, sparse, immutable)
689         elif format == 'incidence_matrix':
690             from .graph_input import from_oriented_incidence_matrix
--> 691             from_oriented_incidence_matrix(self, data, loops=loops, multiedges=multiedges, weighted=weighted)
692
693         elif format == 'DiGraph':

/Users/dcoudert/sage/local/lib/python2.7/site-packages/sage/graphs/graph_input.pyc in from_oriented_incidence_matrix(G, M, loops, multiedges, weighted)
387         L = sorted(set(c.list()))
388         if L != [-1,0,1]:
--> 389             raise ValueError("each column represents an edge: -1 goes to 1")
390         if c[NZ[0]] == -1:
391             positions.append(tuple(NZ))

ValueError: each column represents an edge: -1 goes to 1

```

New commits:

 ​f4b45dc `trac #22985: fix bug`

### comment:3 Changed 5 years ago by tscrim

• Reviewers set to Travis Scrimshaw
• Status changed from needs_review to positive_review

LGTM.

Thanks guys!

### comment:5 Changed 5 years ago by vbraun

• Branch changed from u/dcoudert/22985 to f4b45dc8654be7d0d194448a17dc6689ac7cc730
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.