-
Notifications
You must be signed in to change notification settings - Fork 124
feat : Add VNGraphs.jl stubs for chromatic_number and edge_chromatic_number #507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for now, let's keep the stubs private, so that it is not a breaking change when we decide how to incorporate them with GraphsOptim and GraphsColoring i.e., we will not be including documentation pages and this one should be deleted |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Graph Coloring | ||
|
|
||
| _Graphs.jl_ provides stubs for graph coloring algorithms. High-performance C-backed implementations can be found in add-on packages such as `VNGraphs.jl`. | ||
|
|
||
| ## Index | ||
|
|
||
| ```@index | ||
| Pages = ["coloring.md"] | ||
| ``` | ||
|
|
||
| ## Full docs | ||
|
|
||
| ```@autodocs | ||
| Modules = [Graphs] | ||
| Pages = [ | ||
| "vngraphs_stubs.jl", | ||
| ] | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -454,7 +454,9 @@ export | |
|
|
||
| # planarity | ||
| is_planar, | ||
| planar_maximally_filtered_graph | ||
| planar_maximally_filtered_graph, | ||
| chromatic_number, | ||
| edge_chromatic_number | ||
|
Comment on lines
+458
to
+459
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's not export them |
||
| """ | ||
| Graphs | ||
|
|
||
|
|
@@ -478,6 +480,7 @@ and tutorials are available at the | |
| """ | ||
| Graphs | ||
| include("interface.jl") | ||
| include("vngraphs_stubs.jl") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these stubs are not VNGraphs specific, rather they are for coloring. Maybe we can name the file |
||
| include("utils.jl") | ||
| include("frozenvector.jl") | ||
| include("deprecations.jl") | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||||||||
| # Algorithm stubs for VNGraphs.jl integration | ||||||||||||||||
|
|
||||||||||||||||
| """ | ||||||||||||||||
| chromatic_number(g, timeout=0) | ||||||||||||||||
|
|
||||||||||||||||
| Return the chromatic number of the graph `g`. | ||||||||||||||||
| Note: This is a stub provided by `Graphs.jl`. For a concrete implementation, | ||||||||||||||||
| please use the `VNGraphs.jl` package which provides a fast C implementation | ||||||||||||||||
| via `very_nauty`. | ||||||||||||||||
|
Comment on lines
+7
to
+9
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you are including "notes" use the Documenter.jl note syntax that puts it in a separate panel And let's make the API be
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the |
||||||||||||||||
|
|
||||||||||||||||
| ```julia | ||||||||||||||||
| using VNGraphs | ||||||||||||||||
| chromatic_number(g) | ||||||||||||||||
|
Comment on lines
+11
to
+13
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if you include code in docstrings, make sure it is a doctest, so that it is automatically checked in case it might be outdated. Here we can probably just remove it altogether |
||||||||||||||||
| ``` | ||||||||||||||||
| """ | ||||||||||||||||
| function chromatic_number(g::AbstractGraph, args...; kwargs...) | ||||||||||||||||
| error( | ||||||||||||||||
| "chromatic_number is not implemented in Graphs.jl. " * | ||||||||||||||||
| "Please load VNGraphs.jl to use the very_nauty implementation.", | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+17
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not a big fan of this style of defining functions to be extended. This function will get invalidated when another library gets imported, causing bad user experience (latency for imports/precompilation). I think a cleaner way to do this is to use an error hint defined in the |
||||||||||||||||
| end | ||||||||||||||||
|
Comment on lines
+16
to
+21
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
We want just to declare it as part of the namespace without giving it an implementation. No need to worry about adding error hints even, given that it is private for now |
||||||||||||||||
|
|
||||||||||||||||
| """ | ||||||||||||||||
| edge_chromatic_number(g, timeout=0) | ||||||||||||||||
|
|
||||||||||||||||
| Return the edge chromatic number of the graph `g`. | ||||||||||||||||
| Note: This is a stub provided by `Graphs.jl`. For a concrete implementation, | ||||||||||||||||
| please use the `VNGraphs.jl` package which provides a fast C implementation | ||||||||||||||||
| via `very_nauty`. | ||||||||||||||||
|
|
||||||||||||||||
| ```julia | ||||||||||||||||
| using VNGraphs | ||||||||||||||||
| edge_chromatic_number(g) | ||||||||||||||||
| ``` | ||||||||||||||||
| """ | ||||||||||||||||
| function edge_chromatic_number(g::AbstractGraph, args...; kwargs...) | ||||||||||||||||
| error( | ||||||||||||||||
| "edge_chromatic_number is not implemented in Graphs.jl. " * | ||||||||||||||||
| "Please load VNGraphs.jl to use the very_nauty implementation.", | ||||||||||||||||
| ) | ||||||||||||||||
| end | ||||||||||||||||
|
Comment on lines
+23
to
+41
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same changes to this one |
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -155,6 +155,7 @@ tests = [ | |
| "trees/prufer", | ||
| "experimental/experimental", | ||
| "planarity", | ||
| "vngraphs_stubs", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given that these are kept private, I guess there are no new tests to be added |
||
| ] | ||
|
|
||
| @testset verbose = true "Graphs" begin | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| using Graphs | ||
| using Test | ||
|
|
||
| @testset "VNGraphs Stubs" begin | ||
| g = path_graph(5) | ||
|
|
||
| @test_throws ErrorException chromatic_number(g) | ||
| @test_throws ErrorException edge_chromatic_number(g) | ||
|
|
||
| # Verify the error message contains the suggestion to load VNGraphs.jl | ||
| try | ||
| chromatic_number(g) | ||
| catch e | ||
| @test contains(e.msg, "VNGraphs.jl") | ||
| end | ||
|
|
||
| try | ||
| edge_chromatic_number(g) | ||
| catch e | ||
| @test contains(e.msg, "VNGraphs.jl") | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call them
declared private functions without any methodsinstead ofstubsand point to VNGraphs.jl as one place that happens to implement methods, but do not make it sound like it is meant to be the only one