Skip to content
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

Fixes network layouts where x or y are constant #32

Closed
wants to merge 3 commits into from

Conversation

kippjohnson
Copy link

@kippjohnson kippjohnson commented Oct 16, 2018

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)
}

This scale function can deal with constant columns, returning 0.5 for all values instead of NaN.
Copy and paste mistake earlier
@kippjohnson
Copy link
Author

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 briatte added this to the v0.5.6 – CRAN update milestone Jun 27, 2019
@briatte briatte closed this in 37e9e8a Jun 29, 2019
@briatte
Copy link
Owner

briatte commented Jun 29, 2019

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 37e9e8a.

This is what the fix looks like:

# rescale coordinates if needed (discussed in PR #32)
if (length(unique(nodes$x)) > 1) {
nodes$x <- as.vector(scale(nodes$x, center = min(nodes$x),
scale = diff(range(nodes$x))))
} else {
nodes$x <- 0.5 # constant `x` coordinate
}
if (length(unique(nodes$y)) > 1) {
nodes$y <- as.vector(scale(nodes$y, center = min(nodes$y),
scale = diff(range(nodes$y))))
} else {
nodes$y <- 0.5 # constant `y` coordinate
}

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 added a commit that referenced this pull request Jun 29, 2019
@briatte
Copy link
Owner

briatte commented Jun 29, 2019

Alright, commit 43a7114 does almost exactly what you recommended to do. Thanks again!

@kippjohnson
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants