Opened 4 years ago

Closed 4 years ago

#18530 closed enhancement (fixed)

Simplify generation of some basic graphs

Reported by: dcoudert Owned by:
Priority: minor Milestone: sage-6.8
Component: graph theory Keywords:
Cc: ncohen Merged in:
Authors: David Coudert Reviewers: Nathann Cohen
Report Upstream: N/A Work issues:
Branch: 347e681 (Commits) Commit: 347e681864be2953d5ba0bfbb2b224b4e682d441
Dependencies: Stopgaps:

Description

Avoid calling networkx and then converting to sage graphs for generating very simple graphs (Bull graph, Claw Graph, etc.)

Change History (20)

comment:1 Changed 4 years ago by dcoudert

  • Branch set to public/18530

comment:2 Changed 4 years ago by git

  • Commit set to 0e527304fcef76e8e3abfae639eb1b6892d9924b

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

59fcce0trac #18530: Cycle Graph
91fb0b6trac #18530: Complete Graph
7c07d50trac #18530: Diamond Graph
080d428trac #18530: House Graph
c581ca0trac #18530: House X Graph
6b437aftrac #18530: Star Graph
55a5cbbtrac #18530: Path Graph
842658dtrac #18530: Complete Bipartite Graph
deec0b6trac #18530: Circular Ladder Graph
0e52730trac #18530: Grid 2d Graph

comment:3 Changed 4 years ago by dcoudert

  • Cc ncohen added
  • Status changed from new to needs_review

I did some of them. I don't know if we need to do all of them yet ;)

comment:4 Changed 4 years ago by vdelecroix

  • Status changed from needs_review to needs_work

Hi,

Trac ticket #18530 -> :trac:`18530`

Last edited 4 years ago by vdelecroix (previous) (diff)

comment:5 Changed 4 years ago by git

  • Commit changed from 0e527304fcef76e8e3abfae639eb1b6892d9924b to 8ead714bad28ad0c5c05a7e6ffee76cc9164fd26

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

8ead714trac #18530: review

comment:6 Changed 4 years ago by dcoudert

  • Status changed from needs_work to needs_review

Right.

comment:7 Changed 4 years ago by dcoudert

  • Status changed from needs_review to needs_work

does not pass all tests :( The order of the vertices changed in Grid2dGraph.

Last edited 4 years ago by dcoudert (previous) (diff)

comment:8 Changed 4 years ago by git

  • Commit changed from 8ead714bad28ad0c5c05a7e6ffee76cc9164fd26 to b1a5741414d4588000bbc74a1e6857afc2a11f26

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

2179393trac #18530: better (Circular) Ladder Graph
b1a5741trac #18530: a Path Graph of order 1 has a vertex

comment:9 Changed 4 years ago by git

  • Commit changed from b1a5741414d4588000bbc74a1e6857afc2a11f26 to b0936f38b2d7c04b20f806634e58509c5c424338

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

79d91f5trac #18530: fix some issues
325c978trac #18530: further cleaning
2547d93trac #18530: resolve tests in digraph.py
b0936f3trac #18530: resolve tests in generic_graph.py

comment:10 Changed 4 years ago by dcoudert

  • Status changed from needs_work to needs_review

Thanks to Nathann, I have solve all the problems.

I have not changed all the methods and there is room for further cleaning, but it's a beginning.

comment:11 Changed 4 years ago by ncohen

Helloooooooooo !

I added a commit at public/18530 (since you chose to make the branch public).

Most of the changes I made are superficial:

  • Reduce the number of lines whenever possible
  • Add edges from an iterator instead of a dictionary. Adding edges from a dictionary calls add_edge on an iterator, so it can only be slower

I also fixed a bug with ladder_graph, which was incorrect (plot it, you will see).

Most importantly, if you have never looked at paths, now is the time:

sage: graphs.PathGraph(3).show()
sage: graphs.PathGraph(30).show()
sage: graphs.PathGraph(100).show()

Run those three commands. You will be surprised :-P

Nathann

comment:12 Changed 4 years ago by git

  • Commit changed from b0936f38b2d7c04b20f806634e58509c5c424338 to 9a7caee4d72344cdd107a7bf624971d842c3148b

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

2559809trac #18530: Merged with 6.8.beta2
9a7caeetrac #18530: Reviewer' s commit

comment:13 Changed 4 years ago by dcoudert

Thanks for the corrections.

I had to update some doctests since you changed from Path Graph to Path graph. I had also to update the test on spring_layout in generic_graph.py line 15174.

And yes, the layout of the path is pretty cool ;)

comment:14 Changed 4 years ago by git

  • Commit changed from 9a7caee4d72344cdd107a7bf624971d842c3148b to 347e681864be2953d5ba0bfbb2b224b4e682d441

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

347e681trac #18530: update doc tests in generic_graph.py

comment:15 Changed 4 years ago by ncohen

Hmmmmmm O_o

Actually I did not mean to make that change (I just copy/pasted the first of your two lines) but it seems that all other graphs has a 'graph' and not a 'Graph', so I guess that's fine O_o

Nathnn

comment:16 Changed 4 years ago by ncohen

(and sorry for the broken doctests)

comment:17 Changed 4 years ago by dcoudert

You are welcome ;)

comment:18 Changed 4 years ago by ncohen

  • Reviewers set to Nathann Cohen
  • Status changed from needs_review to positive_review

Wellll, then....

comment:19 Changed 4 years ago by dcoudert

Thanks.

comment:20 Changed 4 years ago by vbraun

  • Branch changed from public/18530 to 347e681864be2953d5ba0bfbb2b224b4e682d441
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.