Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#18785 closed enhancement (fixed)

chang graphs

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

Description

Change History (10)

comment:1 Changed 4 years ago by ncohen

  • Branch set to pubic/18785
  • Status changed from new to needs_review

comment:2 Changed 4 years ago by ncohen

  • Branch changed from pubic/18785 to public/18785
  • Commit set to 65540176492cdad3abe4de47cb11c681ee839acb

New commits:

6554017trac #18785: chang graphs

comment:3 Changed 4 years ago by dcoudert

The patch is OK, but you should add the name of the method to the list of families of graphs (table at the begining of file graph_generators.py).

comment:4 follow-up: Changed 4 years ago by dcoudert

My mistake, it is in the list. However, why not using the same convention than for other graph generators, i.e., ChangGraphs instead of chang_graphs ?

comment:5 Changed 4 years ago by dimpase

perhaps it's time for Sage to get two-graphs and Seidel (aka graph) switching. (Chang graphs are a switching class, see https://en.wikipedia.org/wiki/Two-graph).

comment:6 in reply to: ↑ 4 ; follow-up: Changed 4 years ago by ncohen

Hellooooooooo,

However, why not using the same convention than for other graph generators, i.e., ChangGraphs instead of chang_graphs ?

I followed the "convention" that the code seems to follow about functions which return *several* graphs: fusenes/planar_graph/trees/triangulations/line_graph_forbidden_subgraphs/cospectral_graphs/.... There is not a single "Graphs" but several "graphs" already.

Nathann

comment:7 Changed 4 years ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to positive_review

ok, so then it's good to go.

comment:8 Changed 4 years ago by ncohen

Thanks !

comment:9 Changed 4 years ago by vbraun

  • Branch changed from public/18785 to 65540176492cdad3abe4de47cb11c681ee839acb
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 in reply to: ↑ 6 Changed 3 years ago by dimpase

  • Commit 65540176492cdad3abe4de47cb11c681ee839acb deleted

Replying to ncohen:

Hellooooooooo,

However, why not using the same convention than for other graph generators, i.e., ChangGraphs instead of chang_graphs ?

I followed the "convention" that the code seems to follow about functions which return *several* graphs: fusenes/planar_graph/trees/triangulations/line_graph_forbidden_subgraphs/cospectral_graphs/.... There is not a single "Graphs" but several "graphs" already.

IMHO it still must be Chang (it's a person, whereas cospectral is not!), not chang...

And, by the way, switching: #18972 - not completely done, but still :-)

Note: See TracTickets for help on using tickets.