Opened 4 years ago

Closed 4 years ago

#24051 closed enhancement (fixed)

Issue with edge_colors when plotting undirected multi graph

Reported by: dcoudert Owned by:
Priority: major Milestone: sage-8.1
Component: graph theory Keywords:
Cc: slelievre Merged in:
Authors: David Coudert Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 5fe4c3c (Commits, GitHub, GitLab) Commit: 5fe4c3cf2bea4ee2647209e3ee787605db57432b
Dependencies: Stopgaps:

Status badges

Description

The following example has been reported here. The plot has 4 edges, 2 red and 2 black, while the multi graph has only 2 edges.

sage: G = Graph([(0,1), (0,1)], multiedges=True)
sage: G.plot(edge_colors={"red":[(1,0)]})

Change History (10)

comment:1 Changed 4 years ago by dcoudert

  • Branch set to u/dcoudert/24051
  • Cc slelievre added
  • Commit set to e659503b489cff0c6496809c38dc038ee1adceb6
  • Status changed from new to needs_review

This is certainly not the smartest fix, but it does the job. If someone has a smarter solution, feel free to change the branch.


New commits:

e659503trac #24051: fix reported issue

comment:2 Changed 4 years ago by tscrim

I believe the problem comes from the fact that edges_drawn is being built from the edge_colors does not take into account when the input is out of order. Thus, the black edges get doubly added in the # Add unspecified edges part. So I think more care is needed there in the undirected case. So I feel that this is what needs to be addressed.

comment:3 Changed 4 years ago by git

  • Commit changed from e659503b489cff0c6496809c38dc038ee1adceb6 to 4a3d686539b5f62a974fd62c857260354d7a35ee

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

4a3d686trac #24051: second trial

comment:4 Changed 4 years ago by dcoudert

This is simpler.

comment:5 Changed 4 years ago by tscrim

Because of short-circuiting, you can simply do:

                if ((edge[0],edge[1],edge[2]) not in edges_drawn and
                    ( self._graph.is_directed() or
                      (edge[1],edge[0],edge[2]) not in edges_drawn)
                    ):

comment:6 Changed 4 years ago by git

  • Commit changed from 4a3d686539b5f62a974fd62c857260354d7a35ee to 5fe4c3cf2bea4ee2647209e3ee787605db57432b

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

5fe4c3ctrac #24051: simpler test

comment:7 Changed 4 years ago by dcoudert

You are perfectly right.

comment:8 Changed 4 years ago by tscrim

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

The plots I tested all look good. Thanks.

comment:9 Changed 4 years ago by jmantysalo

Thanks! You may want to look at #13827 too.

comment:10 Changed 4 years ago by vbraun

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