Add package dependency network graph to the blueprint #1001
Conversation
|
Code Coverage SummaryDiff against mainResults for commit: ee66e8b Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 19 suites 11s ⏱️ Results for commit 59d6b4b. ♻️ This comment has been updated with latest results. |
vignettes/blueprint/config.yml
Outdated
| - teal.osprey | ||
| - teal.goshawk | ||
| - teal.modules.clinical | ||
| - teal.modules.helios |
There was a problem hiding this comment.
Question: Do we need teal.modules.helios here or do we add it here after the public release is made? If we only use public repos we don't need GitHub token to access the staged_dependencies.yaml and the pkgdown site will build without any issues.
|
Note that this is quite lengthy process to complete - are we good with that? Especially that it's in CI that got re-triggered quite frequently? I feel that there could be much faster way of getting that information using Also, I really doubt such visualisation on the NEST level will give a lot of value. There are quite a few issues about deps already so this is nothing new I feel. If you want to enhance it more - I would prefer to think more repo-wise than project-wise. I think it could be more actionable. |
|
Agree that |
I can't imagine work without |
Honestly, I am quite surprised seeing this and agree with what @gogonzo said. The staged-deps and CRAN should be independent. CRAN is a deployment repository and staged-deps allows to simulate mono-repo for multiple-repo development purposes. I'm not a big fan of this but it seems that sometimes it's needed - hence: we don't want to remove it (or more precisely: we are not ready to remove it).
OK - I understand. Then let's continue discussing. We can do this two ways:
x <- pkgdepends::new_pkg_deps(c("insightsengineering/teal", "insightsengineering/teal.data", "insightsengineering/teal.code", "insightsengineering/teal.slice"))
x$solve()
#>
#> ✔ Updated metadata database: 4.88 MB in 8 files.
#>
#> ℹ R 4.3 aarch64-apple-darwin20 packages are missing from Bioconductor
#> ℹ Updating metadata database
#> ✔ Updating metadata database ... done
#>
y <- x$get_resolution()[, c("package", "direct", "deps")]
y
#> # A data frame: 213 × 3
#> package direct deps
#> <chr> <lgl> <list>
#> 1 teal TRUE <tbl [50 × 5]>
#> 2 teal.data TRUE <tbl [18 × 5]>
#> 3 teal.code TRUE <tbl [21 × 5]>
#> 4 teal.slice TRUE <tbl [44 × 5]>
#> 5 BH FALSE <tbl [0 × 5]>
#> 6 MASS FALSE <tbl [10 × 5]>
#> 7 Matrix FALSE <tbl [14 × 5]>
#> 8 R.cache FALSE <tbl [6 × 5]>
#> 9 R.methodsS3 FALSE <tbl [3 × 5]>
#> 10 R.oo FALSE <tbl [5 × 5]>
#> # ℹ 203 more rows
y[1, "deps"][[1]]
#> # A data frame: 50 × 5
#> ref type package op version
#> <chr> <chr> <chr> <chr> <chr>
#> 1 R Depends R ">=" "4.0"
#> 2 shiny Depends shiny ">=" "1.7.0"
#> 3 teal.data Depends teal.data ">=" "0.3.0.9017"
#> 4 teal.slice Depends teal.slice ">=" "0.4.0.9027"
#> 5 teal.transform Depends teal.transform ">=" "0.4.0.9010"
#> 6 checkmate Imports checkmate ">=" "2.1.0"
#> 7 jsonlite Imports jsonlite "" ""
#> 8 lifecycle Imports lifecycle ">=" "0.2.0"
#> 9 logger Imports logger ">=" "0.2.0"
#> 10 magrittr Imports magrittr ">=" "1.5"
#> # ℹ 40 more rowsCreated on 2023-12-12 with reprex v2.0.2 |
|
Thank you @pawelru for suggesting a simpler way of fetching the dependencies. |
|
Personally, I don't find this particularly easy to read. I would have to stop thinking about text and focus on this graph for more than a minute to comprehend it OR give it a glance and ignore it and continue reading. Again, this is me personally, maybe other people may have an easier time with it. |
There was a problem hiding this comment.
Six extra dependencies in the Suggests just to make dependencies graph - I don't feel good with this.
Also, I don't think that user/developer learns a lot about the product by knowing what depends on what. This is redundant information IMO. We should picture how these relationships look like (and we do in the vignettes).
|
Above points are valid. Please remove suggested packages not needed anymore as well as try to experiment with layout / direction of the graph and let's see what you came up with. |
|
Sorry about that, removed the residual dependencies from the previous implementation. Updated the diagram to show the edges with "imports" and "depends", Flipped the direction so it makes sense and Improved the layout. |
It most certainly is not. |
|
I meant to say |
I still don't find it useful. I think graph doesn't have any value added for the users. It just visually unattractive map of our products. Map doesn't say anything but "these are our products and this is how we complicated relationships between them". What we should do (and did already):
No one except posit cares that |
|
Hey, sorry for the late input. I also think the diagram is rather more helpful for internal developers and it's not a landing page for end users so I don't think it gives any value for users |
|
@gogonzo Fair enough. I'll close this PR in that case. The value add (if any) does not justify the addition of new dependencies. |
Hey, @insightsengineering/nest-core-dev 👋
What do you think about adding a package dependency network graph in our product map vignette of the blueprint?
It was super helpful for me to understand how different packages connect to each other.
This adds the network diagram to explore the package dependencies of the teal framework to the Technical Blueprint's Product Map.
It seems like more dependencies are added to suggests but only
visNetworkandconfigare the new suggests and the rest of them are already indirect dependencies ofteal.P.S I used the four primary colors from the NEST logo for the node groups 😉