Project object assignment

[hypebright]

I decided to create one app.R file instead of separate ui, server and global files. At the top level of the app.R file, we initiate the proj_obj with:

proj_obj <- get_proj()

Then in the ui part we pass along this proj_obj to the modules. This means that the modules all got new arguments.

On the server side we handle it a bit differently. We get the initiated project, and we put it in the petit r:

  # Top-level reactive values  
  r <- reactiveValues(active_tab = "",
                      model_updated = 0,
                      proj_obj = get_proj())

throughout the server modules we’ll be referencing r$proj_obj, or we will be updating it with r$proj_obj ← get_proj()

The environment stays clean. This is because the top-level of an app.R file is “global” to the Shiny session (and accessible from ui and server), but it doesn’t pollute the user’s global environment. This also means we can get rid of all the assign statements.

Because we’re running a Shiny app in another location, we need to some magic with working directories, so I introduced a this_wd variable that sets the working directory to the project folder. We store this in r too and his is passed along in the get_proj() function each time.

WIP.

[hypebright]

@RichardHooijmaijers I don't think we need this. I can't find any locations the app where we are pulling the data from the global environment. When we do need this data, we check if it's there, but if it's not, we pull it from the RDS files. So I think we can leave this out. At the same we get rid of the global assignments- they needed to go anyway.

[RichardHooijmaijers]

I hope everything is clear.. Added some comments and pushed quite a lot of changes in this PR. Just have a look and see what you think. For me most things are OK now (especially the main reason for this PR; do not use global assignment). Will add some comments to notion

[RichardHooijmaijers]

Not sure why w need magrittr as depend? Seems only %>% is used and is imported in NAMESPACE, right? Could consider using the base R pipe |> for this as it only occurs once in the entire code.
was wondering about the license file, saw LICENCE.md is new. Is it default behavior to ad such a file , e.g. is this not apparent when MIT is specified?

[hypebright]

@RichardHooijmaijers any packages used in the NAMESPACE must be listed in one of Imports or Depends. We can move it to Imports instead. Using the base pipe is a good idea, but you must be aware of your audience since this reduces backwards compatibility (as it also makes the package only compatibility with R >= 4.1). If that's not a problem I would replace it.

As per R CMD check, we needed the LICENSE.md file. It is not send to CRAN though, it is in .Rbuildignore. You can read more about that here: https://r-pkgs.org/license.html#key-files

[RichardHooijmaijers]

Need to revise this folder structure. In the end we need the subfolders "analysis", "data", etc. directly in loc. Noticed the difficulties with this folder structure and found that this is basically caused by the fact that the app was placed in the directory structure, this makes setting the required wd difficult/impossible. Because we don't want the app files in the directory structure, I made a completely new version of run_shinymixr. This now works with the folder structure and makes everything a lot cleaner
I changed this function also, to reflect mostly the original version

[RichardHooijmaijers]

See remark create_proj, I think the folder structure should be different. Will propose some changes here

[RichardHooijmaijers]

would be good to leave out proj_obj from top level. I think we will not need it in UI (checked and likely it is used to get model list, which is updated anyway when tab is switched). Will propose some change here

Also this file can be deleted in the end (see run_shinymixr

[RichardHooijmaijers]

This function is changed because the way run_shinymixr is currently setup. At least dry_run is gone.

[RichardHooijmaijers]

I had some issues with this one... I wanted to add nlmixr2 as import. I was hoping that when running document() it was automatically added to NAMESPACE. This was not the case, so I added it manually, but was removed when running document again. I guess I am doing something wrong here but like to briefly discuss this issue (noticed you added manual imports as well, right?)

[hypebright]

@RichardHooijmaijers the NAMESPACE file should never be edited manually! If you want it to end up there, you need to include it in the roxygen comments of the function that uses the function (at least once in one of your functions), and then run devtools::document(). Alternatively, if you have things that need to be imported "globally", you can use the shinyMixR-package.R file for that. If you add new importFrom statements there, and run devtools::document(), they also ed up in NAMESPACE.

[RichardHooijmaijers]

looks good, let's finish it up!