643 potential removal of dependencies

insightsengineering / teal.modules.general

General Purpose Teal Modules

https://insightsengineering.github.io/teal.modules.general/

Other

9 stars 13 forks source link

643 potential removal of dependencies #663

Closed chlebowa closed 9 months ago

chlebowa commented 9 months ago

Closes #643

m7pr commented 9 months ago

Just to summarize main changes

magrittr::%>% is substittuted with dplyr::%>% hence magrittr is removed from Imports
teal.data is upgraded from Suggests to Imports as teal.data::datanames() is widely used (e.g. srv_front_page)
shinyTree is downgraded from Depends to Imports as it is used in ui_viewer of tm_file_viewer
methods is removed from Suggests
library(nestcolor) is changed to require(nestcolor) in examples so that nestcolor can remain in Suggests and we do not loose all examples (if we apply @examplesIf requireNamespace and nestcolor is not installed in examples)

m7pr commented 9 months ago

Will take a look on my own now.

m7pr commented 9 months ago

MASS::fitdistr is used in srv_distribution of tm_g_distribution but it's only in Suggests - should we upgrade to Imports?
colourpicker::colourInput is used in tm_g_scatterplot but it's only in Suggests - should we upgrade to Imports?
tools::file_ext is used in tm_file_viewer but not in neither Suggests, Imports nor Depends - should we add to Imports?

All 3 packages would be caught if we did not use prefixes but would go with the other school that uses @imporfFrom manually for every foreign function in the package : D

m7pr commented 9 months ago

I actually used your code @chlebowa from here https://github.com/insightsengineering/idr-tasks/issues/632#issuecomment-1708247088 to detect MASS, colourpicker and tools not being added to Imports.

chlebowa commented 9 months ago

tools is an oversight and should be added to Imports, thank you. The other ones are covered by if(requireNamespace), if I am not mistaken.

chlebowa commented 9 months ago

Funny that checks had passed without Imports: tools. Perhaps because tools is always installed as part of r-base?

m7pr commented 9 months ago

tools is an oversight and should be added to Imports, thank you. The other ones are covered by if(requireNamespace), if I am not mistaken.

Sorry that I missed colourpicker and MASS : D they are used in requireNamespace with a Filter and vector input lol

  extra_packages <- c("ggpmisc", "ggExtra", "colourpicker")
  missing_packages <- Filter(function(x) !requireNamespace(x, quietly = TRUE), extra_packages)

m7pr commented 9 months ago

Funny that checks had passed without Imports: tools. Perhaps because tools is always installed as part of r-base?

Maybe, but R sometimes warns about stats or grDevice or methods which are a part of R Base, so not sure tbh.

m7pr commented 9 months ago

Ok, so besides tools, this is good to go. R CMD does not warn about it so I think we can skip it,