briatte / ggnetwork

Geoms to plot networks with ggplot2
https://briatte.github.io/ggnetwork/
146 stars 28 forks source link

Fixes network layouts where x or y are constant #32

Closed kippjohnson closed 5 years ago

kippjohnson commented 5 years ago

If we want to plot a network in a straight line such as 1 --> 2 --> 3 by passing in an xy layout matrix, ggnetwork breaks because the rescaling function used gives NaN. I added a simple scale2() function that deals with the special case where a column is constant by just setting all values in the column = 0.5.

See error example below:

## Create network
edge.list <- matrix(c(1,2,2,3), ncol=2)
net <- network(edge.list, matrix.type='edgelist')

## Create x,y layout matrix:
xy.mat <- matrix(c(1,2,3,1,1,1), ncol=2)
colnames(xy.mat) <- c('x', 'y')

## Does not work
ggplot(net, layout=xy.mat, aes(x = x, y = y, xend = xend, yend = yend)) +
  geom_edges() + geom_nodes()

Problematic lines of code in ggnetwork:::fortify.network() :

nodes$x = scale(nodes$x, center = min(nodes$x), scale = diff(range(nodes$x)))
nodes$y = scale(nodes$y, center = min(nodes$y), scale = diff(range(nodes$y)))
## >>> These return NA when nodes$x or nodes$y are a constant 

Simple fix: change the scale functions above to scale2(), defined here:

scale2 <- function(xx){
    s <- diff(range(xx))
    if(s==0){
      res <- rep(0.5, length.out=length(xx))
    }else{
      res <- scale(xx, center = min(xx), scale = s)
    }
    return(res)
}
kippjohnson commented 5 years ago
checking tests ... ERROR
  Running ‘testthat.R’
Running the tests in ‘tests/testthat.R’ failed.
Last 13 lines of output:
  > library(ggnetwork)
  Loading required package: ggplot2
  > 
  > test_check("ggnetwork")
  ── 1. Failure: load_pkg works (@test-utilities.R#5)  ───────────────────────────
  `expect_warning(load_pkg(-999), "no package")` threw an error with unexpected message.
  Expected match: "install the"
  Actual message: "could not find function \"load_pkg\""

This doesn't seem related to my edits?

briatte commented 5 years ago

Hi @kippjohnson

First of all, all apologies for answering only now.

The problem is related, yet different from, #33.

My impression is that a simpler fix is available: see commit https://github.com/briatte/ggnetwork/commit/37e9e8a9f15dcb7d2f06ad25a692e3591c39b293.

This is what the fix looks like:

https://github.com/briatte/ggnetwork/blob/37e9e8a9f15dcb7d2f06ad25a692e3591c39b293/R/fortify-network.R#L128-L140

Still, your fix has the interesting property of precomputing diff(range(x)), which we need for the rescaling, so it might actually be a bit faster.

I'll refactor the code to see what I come up with. I'll close the PR but add you as a contributor for this.

Thanks again, and so sorry for being very slow to take up your PR!

briatte commented 5 years ago

Alright, commit https://github.com/briatte/ggnetwork/commit/43a71144e6fc6690801e9dac84e688e50673bc96 does almost exactly what you recommended to do. Thanks again!

kippjohnson commented 5 years ago

Thanks for your great work on this package! I was happy to help in whatever small way.