mfasiolo / mgcViz

An R package for interactive visualization of GAM models
https://mfasiolo.github.io/mgcViz/
76 stars 9 forks source link

Could rgl be moved to imports? #75

Closed florianhartig closed 3 years ago

florianhartig commented 3 years ago

While testing code related to https://github.com/florianhartig/DHARMa/issues/309, I have the following problem on Mac:

I believe both is caused by having rgl in import instead of depends, which means that rgl will always be attached once mgcViz is attached or used, instead of only attaching the dll if rgl is actually used.

I am not very happy to introduce these unnecessary hard dependencies in DHARMa. May I ask if there is a particular reason to have rgl in imports instead of depends? Same could actually be asked for qgam and ggplot2, seems not strictly necessary to me to have them in depends?

mfasiolo commented 3 years ago

I am looking into this, switching to an import should not be a problem but I need to run some tests. Overall, the dependency on rgl has been a pain throughout!

florianhartig commented 3 years ago

That would be great. I have a DHARMa branch with the mgcViz dependency ready, but I would not like to merge it into my main branch before this is on CRAN, because the rgl dependency is very persistent / annoying.

I think once this is away from import, rgl should only be loaded once it is actually used, which helps a lot I would think.

florianhartig commented 3 years ago

Hi Matteo, any news on this? I would be ready to switch DHARMa to mgcViz once this is resolved ...

mfasiolo commented 3 years ago

HI Florian, sorry for being so slow! I am trying a more radical solution, that is to make rgl a Suggest so that you will not have to install it at all.

florianhartig commented 3 years ago

Hi Matteo, I think this was solved wit the last CRAN update, right? All my DHARMa checks work fine, so I'm in the process of including the mgcViz dependency with the next DHARMa update on CRAN.

Thanks for your help!

mfasiolo commented 3 years ago

Hi Florian, yes this should be solved now. Fingers crossed that I've not overlooked anything!