search

r-causal / ggdag

:arrow_lower_left: :arrow_lower_right: An R package for working with causal directed acyclic graphs (DAGs)
https://r-causal.github.io/ggdag/
Other
433 stars 28 forks source link

Issue with next version of ggraph (and current ggplot2) #143

Closed thomasp85 closed 7 months ago

thomasp85 commented 7 months ago

Hi

I'm preparing a ggraph release, partly to fix stuff for the new ggplot2 version. My revdepchecks shows issues with ggdag. Part of the test failures are related to ggplot2 and the new guide system, but there is a couple of tests that seems connected to the dag layout though I fail to understand what is going on.

Would you mind take a look to see if it is an issue in dev ggraph or something that should be fixed in ggdag?

thomasp85 commented 7 months ago

Proping a bit more into it, this all seems to be a case of overzealous tests. Whenever you are testing for equivalence of tidy_daggity objects you now get an error because the underlying igraph objects are not the same (they are equivalent though)

I can't say how this used to work, but it would be better to create some more specific tests for this, e.g. by extracting the node and edge data as data frames and compare them.

Since this is, AFAIK, unrelated to ggraph I'll proceed with the submission

thomasp85 commented 7 months ago

So, digging even deeper this is related to a change in ggraph since it now adds the layout coordinates to the graph stored in the layout forcing a copy of the main pointer of the igraph object.

I'll provide a PR that hopefully untangles this change from your unit tests

malcolmbarrett commented 7 months ago

Thanks for digging into this. Sorry for the delay -- was travelling for work.

My tests were actually succeeding here. I do expect them to be truly equal. What it was missing was that I also expect that there is no such attribute attached to the data frame. I lucked out that the igraph diff was complex such that it detected it. In previous versions, that attribute was stripped as a byproduct of some data manipulation. It appears that the result from create_layout() being a tibble has made it more resistant to that in this case.

So, I need to manually strip that attribute and write a test that that is the case.