rigraph
rigraph copied to clipboard
Relative vertex.size + fix in tests + spelling in plot.common.Rd
Following issue https://github.com/igraph/rigraph/issues/161, this PR makes a significan change in the way vertex.size
is processed. In particular, instead of using the scaling (1/200)
as seen throughout the code, plot.igraph
now uses the function i.rescale.vertex
which rescales the vertex size such that the smallest vertex is of size vertex.relative.size[1]
and the largest one of size vertex.relative.size[2]
(vertex.relative.size
is a new parameter that has been added as relative.size
in the list i.vertex.default
and has a default values of c(.01,.025)
[1%,2.5% of the plotting region]).
This new feature has been included to be compatible with rglplot
and tkplot
so all three plotting functions have somewhat same vertex sizes.
A neat example of this new feature is:
igraph_options(plot.layout=layout_nicely)
set.seed(312)
g <- barabasi.game(10)
oldpar <- par(no.readonly=TRUE)
par(mfrow=c(2,2))
set.seed(1);plot(g, main="Default")
set.seed(1);plot(g, vertex.size=degree(g), main="size=degree")
set.seed(1);plot(g, vertex.size=degree(g), vertex.relative.size=c(.05,.1),
main="size=degree,relative.size=c(.05,.1)")
set.seed(1);plot(g, vertex.size=degree(g), vertex.relative.size=c(.025,.2),
main="size=degree,relative.size=c(.025,.2)")
par(oldpar)
which has been included at the end of the file plot.common.Rd
. Furthermore, the changes do not affect significantly examples in the plot.common.Rd
file since by default all vertices occupy 2.5% of the screen.
One important drawback of this PR is that "simple" changes in vertex.size won't be reflected, e.g.
set.seed(1211)
g <- barabasi.game(10)
# These three are equivalent
oldpar <- par(no.readonly=TRUE)
par(mfrow=c(2,3), mar=rep(0,4))
set.seed(1);plot(g)
set.seed(1);plot(g, vertex.size=1)
set.seed(1);plot(g, vertex.size=2)
# But this not
set.seed(1);plot(g, vertex.size=1, vertex.shape="rectangle", vertex.size2=1)
set.seed(1);plot(g, vertex.size=1, vertex.shape="rectangle", vertex.size2=2)
set.seed(1);plot(g, vertex.size=2, vertex.shape="rectangle", vertex.size2=1)
par(oldpar)
Finally, this PR fixes a couple of 'failures' that testthat
showed due to a relatively recent update in which using print
started to be a requirement within 'expectations'.
I understand that this may be a big change but the benefits are greater (in my opinion) since picking vertex sizes will now be more intuitive than before.
Following CONTRIBUTING.md, ping @gaborcsardi @ntamas
In general, this possibility looks quite useful @gvegayon, thanks for contributing! It has taken slightly longer than a week to get back to you on this :wink:
I will have to look at this in a bit more detail, but I had a more general question. The ability to specify relative vertex sizes is definitely useful, however, it is a pity that the ordinary vertex.size
argument no longer works as originally. This will break some existing code of users that simply wanted to create larger (or smaller) vertices. I was wondering whether it would not be possible to do the following:
- Rename
vertex.size2
that you have now tovertex.size
. - Rename the
vertex.size
tovertex.relative.size
to clarify you are specifying a relative size. - Rename
vertex.relative.size
tovertex.relative.scaling
to indicate the scaling of the relative vertex sizes that you have.
That way, users continuing to use vertex.size
would see no difference, but users would be able to specify a (possibly more useful) vertex.relative.size
, where the scaling of the vertex.relative.size
is controlled by vertex.relative.scaling
. Let me know what you think.
No worries wrt to the timing, @vtraag! I see myself reflected on this 😛. Sure, that makes sense. I will take a look at this and try to implement one of your suggested solutions.
If a breaking change is made, this would be an opportunity to think carefully about what the best way is to set vertex sizes. I'll admit I haven't yet looked into how this is done in the R interface yet, but for lack of time, I'll just throw a few Mathematica-inspired ideas here:
- It is useful to set vertex size as a fraction of the shortest edge length, or as a fraction of the distance of the closes two vertices (in case R already has facilities to compute this in better than O(n^2) time).
- It is also useful to set it as a fraction of a bounding region of all vertices (bounding box, bounding disk, whatever)
- Finally, if R plotting generally supports this, it is also useful to set it in device units (e.g. millimeters/points for printing, pixels for screen, etc.) that is independent of plot coordinates. This is what is normally useful for text: the text size should be readable, regardless of whether the plot range is from 0 to 1 or 0 to a million.
Current Aviator status
Aviator will automatically update this comment as the status of the PR changes. Comment
/aviator refresh
to force Aviator to re-examine your PR (or learn about other/aviator
commands).
This pull request is currently open (not queued).
How to merge
To merge this PR, comment /aviator merge
or add the mergequeue
label.
This now has conflicts, my apologies. Are you still interested in the change?
Just solved the conflicts. I haven't tested the code, but it should still work. Will try it out tomorrow.
The vignettes now fail: https://github.com/igraph/rigraph/actions/runs/8446422057/job/23140221959?pr=172#step:7:34 . Can you please take a look?
What about the comments in https://github.com/igraph/rigraph/pull/172#issuecomment-791498917? Do we want to make this a breaking change?
library("igraph")
#>
#> Attaching package: 'igraph'
#> The following objects are masked from 'package:stats':
#>
#> decompose, spectrum
#> The following object is masked from 'package:base':
#>
#> union
g <- make_graph("Zachary")
plot(g)
#> Error in symbols(x = coords[, 1], y = coords[, 2], bg = vertex.color, : invalid symbol parameter
Created on 2024-05-07 with reprex v2.1.0
For some reason in this reprex vertex.size becomes a vector of NA, that is passed to symbols()
. Any idea what could be wrong?