Possible 'declutter' fragility
Closed this issue · 8 comments
Thanks for adding declutter in 0.4.0 as per #27 -- really appreciate it. But are there limits to how far it can go?
> plot_dependencies(deepdep("RcppArmadillo", 2, dependency_type="all"), show_stamp=FALSE, declutter=TRUE)
> plot_dependencies(deepdep("RcppArmadillo", 3, dependency_type="all"), show_stamp=FALSE, declutter=TRUE)
Error in simple_es_index(x, ii) : Unknown edge selected
> The first call works great and as expected. Trying to push it further seems to stretch my luck. Thoughts?
Thoughts are that our testing game is awful and we haven't taken time to step it up since two years ago, so bugs can swim undisturbed in the ocean of deepdep code. I'm gonna fix it and finally cover the code with tests much more tightly.
Well 'new feature' is just a fancy misspelling of bug ... Kidding aside, this is complex. I think I also saw a plot that wasn't quite right:
plot_dependencies(deepdep("dplyr", 2, dependency_type="all"), show_stamp=FALSE, declutter=TRUE)This ends up with an edge between Rcpp and generics but there is none... Maybe the label should be RSQLite ?
> dd <- deepdep("dplyr", 2, dependency_type="all")
> dd[ dd$name == "Rcpp", ]
origin name version type origin_level dest_level
406 lobstr Rcpp <NA> Imports 1 2
408 lobstr Rcpp <NA> LinkingTo 1 2
458 RSQLite Rcpp >= 1.0.7 Imports 1 2
460 RSQLite Rcpp <NA> LinkingTo 1 2
> Thanks a lot for reporting bugs, they'll make great test cases! I wouldn't have suspected the latter problem, shows how you have to test everything, no exceptions. I expect it may take a few days to fix and cover with tests, but I hope I can get this working before holidays.
Partially good news, the plot never had Rcpp linking to generics, it was just so that RSQLite hid perfectly under generics label. In fact, all these visible connections link to RSQLite. Which is a problem on its own, but at least the one we already know of.
I believe I know the reason of the original issue, it's because the code simply removes Suggests and Enhances edges, but doesn't remove edges originating from this removed dependency, oftentimes resulting in disconnected dependency graph. The code will have to repeatedly traverse a deepdep object, removing packages that aren't dependencies of something anymore. Perhaps it might be useful to create a temporary table of layer numbers packages are placed on to know which dependencies to remove in the next step.
Your first example seems to be working now on development branch, expect an update soon.
Very nice! (One micro nag is that reticulate and RcppArmadillo almost 'touch' so we do not need the edge type. )
Ahh, higher resolution would take care of this one :)