cynkra / dm

Working with relational data models in R
https://dm.cynkra.com
Other
497 stars 50 forks source link

`DiagrammeR` package is a dependency that is not automatically loaded with `dm` #1735

Open DOH-RPS1303 opened 1 year ago

DOH-RPS1303 commented 1 year ago
install.packages("dm")

dm <- dm_nycflights13()

dm

Fine up to here:

── Metadata ──────────────────────────────────────────────────
Tables: `airlines`, `airports`, `flights`, `planes`, `weather`
Columns: 53
Primary keys: 4
Foreign keys: 4

Here's where the issue happens:

dm %>%
+   dm_draw()

Which returns:

Error:
! `dm_draw()` needs the 'DiagrammeR' package. Do you need `install.packages("DiagrammeR")` ?
Run `rlang::last_error()` to see where the error occurred.

I already solved this problem for myself by installing the package, but given that this is a necessary dependency for a pretty big part of this package and that it's one of the example code snippets, it'd be nice to have DiagrammeR automatically install with dm

krlmlr commented 1 year ago

Thanks. This is difficult because CRAN limits the number of packages we can import to 20. We're at 17 now. Theoretically, there's room, and I agree that DiagrammeR might be just important enough to warrant inclusion.

The other issue is that it's quite a heavy dependency, but so are many other packages we're already importing.

Would you like to contribute a PR?

pawelru commented 1 year ago

Hello. Recently I also end-up with issues regarding DriagrammeR and DiagrammerRsvg when trying to document my package.

My use case is that I do have library("dm") in my package vignette which results into simiar error message during R CMD BUILD. This is because of this: https://github.com/cynkra/dm/blob/v1.0.5/R/pkgdown.R#L10-L23. My request then is: can it be checked only when calling dm_draw() to allow draw-free usage of dm in vignettes?

Let me also share my 2c to the OP request. This is not for DiagrammeR itself but rather for DiagrammeRsvg but both are highly coupled. I have to start with a disclaimer that I am working in pharma and our packages (together with its hard(!) dependencies) has to be validated. It's a broad term and I don't want to go into the details but essentially that means that a package has to meet certain criteria to be accepted to the pool of packages available for use. DiagrammeRsvg has 0% coverage and is not actively maintained which is a no-go for decision makers. Luckily right now it is part of soft dependencies of dm so that's not an issue. If we promote it into to became hard dep then dm package might face some problems with adaptation in pharma field. dm is a very nice package and I just want you to be aware when deciding about hard deps.

krlmlr commented 1 year ago

Thanks. I don't follow: register_pkgdown_methods() has an effect only if the IN_PKGDOWN environment variable is set. How does that break R CMD build ?

Good catch about validation. Has DiagrammeR been validated at some point?

pawelru commented 1 year ago

Thanks. I don't follow: register_pkgdown_methods() has an effect only if the IN_PKGDOWN environment variable is set. How does that break R CMD build ?

I am sorry, I confused CI steps. You are definitely right - this is a pkgdown step that failed for me - not R CMD BUILD. Nevertheless, my request stayed the same - to allow draw-free usage without aforementioned packages.

Has DiagrammeR been validated at some point?

Not until now. At least within an internal env that I am working on (maybe it was not requested up until now?).

krlmlr commented 1 year ago

This fell through the cracks, I didn't see an action item here. I believe you can use dm without installing DiagrammeR. What fails for you?