Improve arg syntax for `gplot` and cleanup repo #180

hdavid16 · 2022-07-27T01:29:06Z

-breaking change: linetype changed from String to Symbol
-breaking change: default for edgelabel is nothing (not []) to make consistent with nodelabel default
-remove all caps args (Fix #177)
-improve some of the code in gplot (use isnothing, etc.)
-move layouts to layouts.jl
-add community_layout to readme
-remove deprecations
-remove shape.jl and pienode.jl (not used anywhere)

-distinguish args, kwargs -add missing kwargs

clarify that Cairo is required to visualize in vscode

Checking for System OS was outdated

Now matches gadfly.jl (https://github.com/GiovineItalia/Gadfly.jl/blob/master/src/open_file.jl)

-allow gplothtml to accept same arguments as gplot -allow x/y-locs to be of different type (<: Real)

and minor changes to plots.jl update README

-breaking change: linetype changed from String to Symbol -breaking change: default for edgelabel is nothing (not []) to make consistent with nodelabel default -remove all caps args -improve some of the code in gplot (use isnothing, etc.)

codecov · 2022-07-27T01:34:24Z

Codecov Report

Merging #180 (1a945c4) into master (52f4aae) will increase coverage by 3.81%.
The diff coverage is 8.75%.

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
+ Coverage   32.92%   36.73%   +3.81%     
==========================================
  Files           9        4       -5     
  Lines         565      471      -94     
==========================================
- Hits          186      173      -13     
+ Misses        379      298      -81

Impacted Files	Coverage Δ
src/GraphPlot.jl	`100.00% <ø> (ø)`
src/layout.jl	`19.32% <0.00%> (-29.10%)`	⬇️
src/plot.jl	`58.66% <81.25%> (-3.98%)`	⬇️
src/lines.jl	`52.22% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us.

fix background color

-use `iszero` -remove `stroke(nothing)` or `fill(nothing` as `nothing` is already the default for these functions in Compose.jl

-move layouts to layouts.jl -add community_layout to readme -remove deprecations -remove shape.jl and pienode.jl (not used anywhere)

Multiple dispatch was messing up because types were not specified for spring layout and `kws` not preceeded by `;`

simonschoelly · 2022-07-27T15:23:53Z

src/layout.jl

@@ -284,3 +284,288 @@ function _spectral(A::SparseMatrixCSC)
    index = sortperm(real(eigenvalues))[2:3]
    return real(eigenvectors[:, index[1]]), real(eigenvectors[:, index[2]])
 end
+
+# This layout algorithm is copy from [IainNZ](https://github.com/IainNZ)'s [GraphLayout.jl](https://github.com/IainNZ/GraphLayout.jl)


Is there any advantage in moving this function? Maybe it would be easier to read if we keep it in a separate file.

I figured that since we had a layout.jl file with several layouts, I'd just add it there. However, we could have a layout folder and have a separate file for each layout. What are your thoughts?

simonschoelly · 2022-07-27T15:26:29Z

src/plot.jl

-    end
+    linetype = :straight,
+    outangle = π / 5,
+    backgroundc = nothing) where {T <:Integer, R1 <: Real, R2 <: Real}


I think it would be easier to review/write tests, if new features where in a separte PR

Yes. I put it in a PR, but then built further from that one...oops. I'll wait for the background color PR to get merged.

removed since this is done in a separate PR

simonschoelly · 2022-07-27T15:27:53Z

src/plot.jl

 """
 function gplot(g::AbstractGraph{T},
    locs_x_in::Vector{R1}, locs_y_in::Vector{R2};
    nodelabel = nothing,
    nodelabelc = colorant"black",
    nodelabelsize = 1.0,
-    NODELABELSIZE = 4.0,
+    max_nodelabelsize = 4.0,


Totally agree, that we should change this, but I think we should add deprecation warnings for the older functions (that we can eventually remove), or at least produce a meaningful error message.

ok. will do

simonschoelly · 2022-07-27T15:29:36Z

src/plot.jl

+    outangle = π / 5,
+    backgroundc = nothing) where {T <:Integer, R1 <: Real, R2 <: Real}
+
+    @assert length(locs_x_in) == length(locs_y_in) == nv(g) "Position vectors must be of the same length as the number of nodes"


We should only use @assert in situations that we expect that they are always true, as it might be removed by a compiler optimization: https://docs.julialang.org/en/v1/base/base/#Base.@assert

I see. I'll use the if -> error form for these.

simonschoelly · 2022-07-27T15:31:09Z

src/plot.jl

-    if max_nodestrokelw > 0.0
-        max_nodestrokelw = EDGELINEWIDTH / max_nodestrokelw
-        nodestrokelw *= max_nodestrokelw
+    if !iszero(max_nodestrokelw)


Would testing for max_nodestrokelw not make more sense in case that max_nodestrokelw is negative?

We are just catching for divide by 0 errors here I think. Passing negative values should work, they just don't get displayed. However, we can check for all inputs being positive when we do the other checks at the beginning of the function if that would be better.

simonschoelly · 2022-07-27T15:33:43Z

src/plot.jl

-    if !isempty(edgelabel) && length(edgelabel) != NE
-        error("Must have one label per edge (or none)")
-    end
+    linetype = :straight,


Also here we should add a deprecation warning, if one uses a String

yes. will do

hdavid16 · 2022-07-27T16:23:26Z

This PR also fixes #148

This reverts commit 67bd950.

Remove commits that are in separate PRs This reverts commit d4a2ae4ba42a701ebe49865d2addf212c54f7bd7.

.

hdavid16 added 10 commits July 21, 2022 17:20

Improve Readme and docstring

3368e89

-distinguish args, kwargs -add missing kwargs

fix JuliaGraphs#166

febb53b

clarify that Cairo is required to visualize in vscode

Fix JuliaGraphs#175 to enable plotting to html

9bffdd9

Checking for System OS was outdated

Document the use of gplothtml in README

080a1a4

Update open_file

3d32d07

Now matches gadfly.jl (https://github.com/GiovineItalia/Gadfly.jl/blob/master/src/open_file.jl)

Minor changes:

2c64095

-allow gplothtml to accept same arguments as gplot -allow x/y-locs to be of different type (<: Real)

add TagBot to repo

222fb26

Merge branch 'JuliaGraphs:master' into master

dfcdf19

Fix JuliaGraphs#172. set background color (backgroundc kwarg):

14d62b4

and minor changes to plots.jl update README

hdavid16 added 3 commits July 27, 2022 00:32

clarify docstring

5bc0ba3

fix background color

minor changes:

ef373d5

-use `iszero` -remove `stroke(nothing)` or `fill(nothing` as `nothing` is already the default for these functions in Compose.jl

clean up repo:

4d574e0

-move layouts to layouts.jl -add community_layout to readme -remove deprecations -remove shape.jl and pienode.jl (not used anywhere)

hdavid16 changed the title ~~Improve arg syntax for gplot~~ Improve arg syntax for gplot and cleanup repo Jul 27, 2022

Fix JuliaGraphs#154

67bd950

Multiple dispatch was messing up because types were not specified for spring layout and `kws` not preceeded by `;`

simonschoelly reviewed Jul 27, 2022

View reviewed changes

make default consistent with gplot notation (pi/9)

22cddeb

hdavid16 added 4 commits August 2, 2022 09:32

Revert "Fix JuliaGraphs#154"

6f4597f

This reverts commit 67bd950.

Revert "bug fix"

40e21be

Remove commits that are in separate PRs This reverts commit d4a2ae4ba42a701ebe49865d2addf212c54f7bd7.

Revert "bug fix"

f4f375f

Remove commits that are in separate PRs This reverts commit d4a2ae4ba42a701ebe49865d2addf212c54f7bd7.

no change

1a945c4

.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve arg syntax for `gplot` and cleanup repo #180

Improve arg syntax for `gplot` and cleanup repo #180

hdavid16 commented Jul 27, 2022 •

edited

Loading

codecov bot commented Jul 27, 2022 •

edited

Loading

simonschoelly Jul 27, 2022

hdavid16 Jul 27, 2022

simonschoelly Jul 27, 2022

hdavid16 Jul 27, 2022

hdavid16 Aug 2, 2022

simonschoelly Jul 27, 2022

hdavid16 Jul 27, 2022

simonschoelly Jul 27, 2022

hdavid16 Jul 27, 2022

simonschoelly Jul 27, 2022 •

edited

Loading

hdavid16 Jul 27, 2022

simonschoelly Jul 27, 2022

hdavid16 Jul 27, 2022

hdavid16 commented Jul 27, 2022

Improve arg syntax for gplot and cleanup repo #180

Are you sure you want to change the base?

Improve arg syntax for gplot and cleanup repo #180

Conversation

hdavid16 commented Jul 27, 2022 • edited Loading

codecov bot commented Jul 27, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonschoelly Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hdavid16 commented Jul 27, 2022

Improve arg syntax for `gplot` and cleanup repo #180

Improve arg syntax for `gplot` and cleanup repo #180

hdavid16 commented Jul 27, 2022 •

edited

Loading

codecov bot commented Jul 27, 2022 •

edited

Loading

simonschoelly Jul 27, 2022 •

edited

Loading