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

bind_graphs() for list of tbl_graph objects has broken #187

Open
jackWest9731 opened this issue Jan 11, 2024 · 1 comment
Open

bind_graphs() for list of tbl_graph objects has broken #187

jackWest9731 opened this issue Jan 11, 2024 · 1 comment

Comments

@jackWest9731
Copy link

jackWest9731 commented Jan 11, 2024

I see that allowing an input of a list of graphs to bind_graphs() was implemented from issue #88 , and I have been using this feature. After updating the package I have run into an issue when trying to do this. For example:

graph <- create_notable('bull')
new_graph <- create_notable('housex')
graphs <- list(graph, new_graph)
 
bind_graphs(graphs)

Returns an error

Error in UseMethod("unfocus") : no applicable method for 'unfocus' applied to an object of class "list"

I would find it useful if this feature was reimplemented please. Thank you for the work that's been put into this package

@davidskalinder
Copy link

I just ran into this. It looks like the bug was introduced at d88b167 by this line, which calls unfocus() at the top of the function in an attempt to make sure that the new focus() functionality doesn't mess things up; but unfocus can't handle bare lists, so it throws an error.

I assume the fix is to put the unfocus() call inside the if conditions that immediately follow the current call and that handle the bare list functionality. But I don't understand focus() very well yet, so I'll hold back on submitting a PR that might cause some new problem.

In the meantime, I think the bug can be worked around fairly painlessly by using do.call() (or rlang::exec()) to pass the list to bind_graphs() elementwise.

Here's a reprex showing the workaround using the example from the docs and above:

library(tidygraph)
#> 
#> Attaching package: 'tidygraph'
#> The following object is masked from 'package:stats':
#> 
#>     filter
graph <- create_notable('bull')
new_graph <- create_notable('housex')
graphs <- list(graph, new_graph)
 
example_from_docs <- graph %>% tidygraph::bind_graphs(new_graph)
workaround <- do.call(bind_graphs, graphs)
igraph::identical_graphs(example_from_docs, workaround)
#> [1] TRUE

Created on 2024-05-20 with reprex v2.1.0

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

No branches or pull requests

2 participants