Opened 6 years ago
Closed 6 years ago
#13862 closed enhancement (fixed)
Split graph_generators into several files
Reported by: | ncohen | Owned by: | jason, ncohen, rlm |
---|---|---|---|
Priority: | major | Milestone: | sage-5.6 |
Component: | graph theory | Keywords: | |
Cc: | dcoudert | Merged in: | sage-5.6.beta2 |
Authors: | Nathann Cohen | Reviewers: | David Coudert |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
This tickets addresses the fact that Jeroen is not happy with the length of graph_generators.py
:-P
It splits the file into new ones, added in the graph/generators/ folder, according to the classifiation of the methods that appears in the module's documentation.
APPLY:
Attachments (7)
Change History (23)
comment:1 Changed 6 years ago by
- Cc dcoudert added
- Component changed from PLEASE CHANGE to graph theory
- Owner changed from tbd to jason, ncohen, rlm
- Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 6 years ago by
- Description modified (diff)
Changed 6 years ago by
comment:3 Changed 6 years ago by
What this additional (trac_13862-cleaning_and_moving.patch) patch does :
- Moves
_circle_embedding
and_line_embedding
tograph_plot.py
. These methods are now imported at the beginning of generators/* modules that need need. - Removes LCFGraph from the list of basc graphs (this methods creates a family of graphs). It was already in the list of "families" graphs.
- Moves the Harary Graph to the "families" file. Same thing here : there are many Harary graphs.
- Moves
DorogovtsevGoltsevMendesGraph
tofamilies.py
. Not worth creating an independent "pseudofractal" module at the moment. - Moves
IntervalGraph
to families. This one was not liste among the "families", which is why it was still ingraph_generators.py
after the first patch. - Adds the Nauru Graph to the list of small graphs.
Some work on the doc is still needed.
Nathann
Changed 6 years ago by
comment:4 Changed 6 years ago by
Apply trac_13862.patch, trac_13862-cleaning_and_moving.patch
comment:5 Changed 6 years ago by
- Description modified (diff)
comment:6 Changed 6 years ago by
- Description modified (diff)
What this new patch (trac_13862-doc.patch) does :
- Moves the "Usage" part of the graph_generators module's doc to graph_plot, as it explains how graphs are displayed.
- *HEAVY* rewrite of the fuctions lists at the top of graph_generators. Now the methods are listed in 3xN tables, so that they are easier to browse. This required to define an (ugly) helper function in
graph_generators.py
. But the lists at the top of that file are now easier to read (both in html and insage.graphs.graph_generators?
) and easier to update (just add a string in a list). NauruGraph
isadded to the list of small graphs
This patch's almost ready.
Nathann
Changed 6 years ago by
comment:7 Changed 6 years ago by
- Description modified (diff)
What this patch does (trac_13862-code_reformatting.patch):
- Removes all occurrences to
self
, as the former methods are now functions and belong to no class. - Adds a line of doc to all new files, saying that their documentation actually appears in graph_generators
Lots of code formatting :-P
(only one patch left)
Nathann
Changed 6 years ago by
comment:8 Changed 6 years ago by
- Description modified (diff)
- Status changed from new to needs_review
What this patch does (trac_13862-moves_3_constructors.patch):
- Moves
CompleteGraph
,CompleteBipartiteGraph
andCompleteMultipartiteGraph
fromfamilies.py
tobasic.py
- Adds
CompleteMultipartiteGraph
to the lists, as it did not appears beforeO_o
And nooooow, this ticket is ready to be reviewed !!! :-P
Nathann
comment:9 Changed 6 years ago by
- Status changed from needs_review to needs_work
Hello,
the long tests fail on families.py
********************************************************************** File "/path-to-sage/sage-5.6.beta0/devel/sage-myclone/sage/graphs/generators/families.py", line 1815: sage: G = graphs.RingedTree(5) Exception raised: Traceback (most recent call last): File "/path-to-sage/sage-5.6.beta0/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/path-to-sage/sage-5.6.beta0/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/path-to-sage/sage-5.6.beta0/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_25[2]>", line 1, in <module> G = graphs.RingedTree(Integer(5))###line 1815: sage: G = graphs.RingedTree(5) File "/path-to-sage/sage-5.6.beta0/local/lib/python/site-packages/sage/graphs/generators/families.py", line 1845, in RingedTree from sage.graphs.graph_plot import GraphGenerators ImportError: cannot import name GraphGenerators
I have run tests only on the graph directory. I don't know if this patch could impact other modules.
Also, you should add possible dependencies with other patches e.g., #13809.
Changed 6 years ago by
Changed 6 years ago by
Changed 6 years ago by
comment:10 Changed 6 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
God... I ran tests on these files a crazy amount of times but I forgot long tests :-P
Updated ! I cheated a bit by first removing #13809 then adding it again, but this set of patch is.... HEAVY to work with :-P
Nathann
Apply trac_13862-unapply_13809.patch, trac_13862.patch, trac_13862-cleaning_and_moving.patch, trac_13862-doc.patch, trac_13862-code_reformatting.patch, trac_13862-moves_3_constructors.patch, trac_13862-reapply_13809.patch
comment:11 Changed 6 years ago by
- Dependencies set to #13809
- Status changed from needs_review to positive_review
Install OK, doctest OK including long tests, and docbuild OK. For me the patch is good to go!
I have added the dependency to the patch description.
This is definitely a huge patch, but it has to be done. Good job.
comment:12 Changed 6 years ago by
Wow. Glad to see that :-D
Thaaaaaaaaaaaaankss !!!
Nathann
comment:13 Changed 6 years ago by
- Dependencies #13809 deleted
- Description modified (diff)
comment:14 Changed 6 years ago by
- Reviewers set to David Coudert
comment:15 Changed 6 years ago by
Apply trac_13862.patch, trac_13862-cleaning_and_moving.patch, trac_13862-doc.patch, trac_13862-code_reformatting.patch, trac_13862-moves_3_constructors.patch
comment:16 Changed 6 years ago by
- Merged in set to sage-5.6.beta2
- Resolution set to fixed
- Status changed from positive_review to closed
This patch creates the following files and moves into them the methods from
GraphGenerator
that are associated to it in the list at the top of the graph_generators module documentation.This patch passes all tests, but is not clean yet and lacks documentations. Others will follow.
Nathann