Opened 5 years ago

Closed 5 years ago

#22985 closed defect (fixed)

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

Reported by: was Owned by:
Priority: trivial Milestone: sage-8.0
Component: graph theory Keywords:
Cc: Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: f4b45dc (Commits, GitHub, GitLab) Commit: f4b45dc8654be7d0d194448a17dc6689ac7cc730
Dependencies: Stopgaps:

Status badges

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...

Change History (5)

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:

f4b45dctrac #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.

comment:4 Changed 5 years ago by was

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.