search

SPRFMO / jjmr

R package for assessment model
2 stars 5 forks source link

NAMESPACE, which functions should be exported #6

Open arni-magnusson opened 6 years ago

arni-magnusson commented 6 years ago

Currently, the code contains @export flags for some of the commonly used functions. These flags indicate which functions should be made available to the user. Functions that are not exported are internal.

This @export flags are currently ignored, since the NAMESPACE file simply exports all functions, including all the internal functions not intended for the end user. It also turns out that there are too few @export flags in the code. Many functions that need to be exported do not have @export flags.

Some of the functions that should perhaps have @export flags include: plot.jjm.diag, plot.jjm.output, plot.jjm.retro, print.jjm.config, print.jjm.diag, print.jjm.output, print.summary.jjm.config, print.summary.jjm.diag, print.summary.jjm.output, readJJMConfig.default, readJJMConfig.jjm.output, report.jjm.diag, report.jjm.output, runJJM.default, runJJM.jjm.config, runJJM.jjm.output, summary.jjm.config, summary.jjm.diag, summary.jjm.output, writeJJM.jjm.output.

The best solution would be to add @export flags to all functions to be exported, and use Roxygen to create the NAMESPACE file. Until then, the current NAMESPACE file simply exports all functions.

The R Core guidelines for writing packages (https://cran.r-project.org/doc/manuals/r-release/R-exts.html) state that "packages which use internal objects extensively should not export those objects from their namespace" and that exporting all functions is "not recommended for production code". The jjmR has a large number of (around 100?) internal functions that are currently exported.

(This is a low-priority issue, but after putting it together, I thought it would be better to leave the above list of "should-probably-be exported" functions here, rather than on my hard drive or the recycle bin.)

roliveros-ramos commented 6 years ago

It should work now, all the imports are added in aaa-.R. DESCRIPTION file has been modified properly.
It shouldn't break anything, should not...

jimianelli commented 1 year ago

for the modification to change the factor order I invoked the R package "forecast" function fct_rev(), But description file wasn't updated and currently will throw an error (corrected if "library(forcats)"