You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In the package we have an import dplyr and then importFrom dplyr . We should agree whether is it worth the effort to import from dplyr all the separate functions or not. In any case a clean up is necessary. The pros for having the importFrom are:
documentation
clear dependencies
The cons:
extra work
Additionally we should not import the %>% from dplyr, considering that we are importing %<>% from magrittr.
A similar cleanup is needed for the app part. In the server we have library(dplyr) and library(magrittr) despite using everywhere else the notation with ::. magrittr functions are used in only two places without the ::. dplyr is used more extensively for the data.frame manipulation for the plots.
The pros of using the :: notation:
being consistent across the app
do not have unnecessary dependencies
The cons:
extra effort
A separate but related discussion is given by the separation between package and app functionalities. In this view the data.frame manipulation for the plots should be moved out of the server and become a function in the package. On the other hand the function withModalSpinner.R should be moved into the server, as it belongs to the app and not the package.
Pros of having the withModalSpinner.R in the app:
keep the separation between package and app
Pros of having the withModalSpinner.R in the package:
good documentation and examples.
Another option would be to create a small public package with shiny utilities, place the spinner there and load the package in SmaRP app.
In case of having the withModalSpinner.R in the package the documentation should be adapted accordingly (e.g. the shiny function showModal should be imported.
Additional notes:
In the report there are library(reshape2) and library(kableExtra) without any reshape2 or kableExtra functions being used.
The text was updated successfully, but these errors were encountered:
We should actually thinking of going for an all-in-package approach, including server and ui in the package itself, i.e. moving all the app code from inst/ to R/. This approach has several advantages and was a recurrent topic in the community over the last year or so, and was pitched a lot at useR! 2019.
In the package we have an
import dplyr
and thenimportFrom dplyr
. We should agree whether is it worth the effort to import fromdplyr
all the separate functions or not. In any case a clean up is necessary. The pros for having theimportFrom
are:The cons:
Additionally we should not import the
%>%
fromdplyr
, considering that we are importing%<>%
frommagrittr
.A similar cleanup is needed for the app part. In the
server
we havelibrary(dplyr)
andlibrary(magrittr)
despite using everywhere else the notation with::
.magrittr
functions are used in only two places without the::
.dplyr
is used more extensively for thedata.frame
manipulation for the plots.The pros of using the
::
notation:The cons:
A separate but related discussion is given by the separation between package and app functionalities. In this view the
data.frame
manipulation for the plots should be moved out of theserver
and become a function in the package. On the other hand the functionwithModalSpinner.R
should be moved into the server, as it belongs to the app and not the package.Pros of having the
withModalSpinner.R
in the app:Pros of having the
withModalSpinner.R
in the package:Another option would be to create a small public package with shiny utilities, place the spinner there and load the package in SmaRP app.
In case of having the
withModalSpinner.R
in the package the documentation should be adapted accordingly (e.g. theshiny
functionshowModal
should be imported.Additional notes:
In the report there are
library(reshape2)
andlibrary(kableExtra)
without anyreshape2
orkableExtra
functions being used.The text was updated successfully, but these errors were encountered: