teal icon indicating copy to clipboard operation
teal copied to clipboard

Example skeleton project - DO NOT MERGE

Open nikolas-burkoff opened this issue 3 years ago • 11 comments

DRAFT PR not to be merged FyI @gogonzo @pawelru @kumamiao

To use you may need to install teal a few times for RStudio to get it working.

This PR (unmerged) closes https://github.com/insightsengineering/NEST-roadmap/issues/59

We now have:

image image

image

Or if you chose not to have cdisc: image

Clearly if we are doing this for real then

  1. I still suggest it goes in teal rather than having to install a new package
  2. The package_skeleton function can be called from other IDEs
  3. We'd want to structure the package_skeleton function in a nicer way and output a proper structure not just a single app.R file
  4. We can add some other options to the UI of creating the project as well e.g. use renv, add folder for custom modules etc.

You could even have options to create specific apps i.e. see https://github.com/insightsengineering/NEST-roadmap/issues/9#issuecomment-1172392546 - although they wouldn't live in teal itself though could live in teal.gallery @dinakar29

nikolas-burkoff avatar Jul 21 '22 09:07 nikolas-burkoff

This looks great, @nikolas-burkoff!

You could even have options to create specific apps

Skeleton apps derived from sample apps on teal.gallery sounds like a great idea. We should consider that in the near future. Let me add that as an IDR task, as it seems pretty straightforward.

cicdguy avatar Jul 21 '22 11:07 cicdguy

The addon looks good but the content is not there yet (however it's a draft so not sure if I can focus on it at all). For sure I would go away from single-file apps in favour of more sophisticated multiple files. The content is to-be-decided (researched). Feel free to to have a look what's in already archived tealApp project/repo somewhere. I also think that it would be better/easier to have scripts (in inst/) as files than scripts as character string.

I like addon because it promotes good practice of using "New Project" (at least from RStudio). I dislike addon as the configuration possibilities is very limited. I can imagine that the number of config controls would grow (e.g. use testing, use logging and if yes - what log type or even custom module selector as pointed by Dawid) and we might want to group them, make it optional / conditional, etc. This is not possible. I am not yet sure if that's bad - maybe it's good to have it super simple - but I see it as a limitation for the future.

pawelru avatar Jul 21 '22 13:07 pawelru

@kumamiao @donyunardi we have not decided exactly what is needed here. If addin should be a single file or multiple files. There was proposition of having data.R, modules.R sourced from the app.R but wasn't formally confirmed. We suppose to promote good practices but I don't know if these are good practices. Also, we probably should initialize with empty www/css, www/js. Am I missing something?

Additionally, I think that it shouldn't be a separate package because users shouldn't be bothered to install a new package from github. I think we should rather keep it in teal. Alternatives are not good enough, because if we make a new independent package hosted on github then users will be requested to install this. Package will not be able to be installed on CRAN because it's a utility package, which means it can't be in teal/Suggests.

gogonzo avatar Aug 01 '22 12:08 gogonzo

@nikolas-burkoff mentioned this earlier on creating a custom_modules folder, which I thought was a good idea. Another one that comes to mind is to create a folder to store custom functions.

I think separating and sourcing data.R and modules.R from app.R is a good idea. This will make the code more organized and easier to debug. And, while we're on this topic, when I looked around in Roche GitHub, I noticed people's modules= code was long due to the number of parameters for each module and this makes it hard to see the organization of the modules.

What do you think if we separate the module call into its own file under a separate folder? Here's to illustrate what I meant:

app.R

library(teal)

source("data.R")
source("modules.R")
lapply(list.files("module", full.names = TRUE), source)

app <- init(
  data = data,
  modules = modules,
  header = "My first teal application"
)

if (interactive()) {
  runApp(app)
}

data.R

data <- teal_data(
  dataset("IRIS", iris),
  dataset("MTCARS", mtcars)
)

modules.R

modules <- modules(
  x1,
  x2,
  modules(
    label = "Nested",
    x1,
    x2
  )
)

module/module1.R

x1 <- example_module()

module/module_2.R

x2 <- example_module()

The structure would look like this:

/
-- app.R
-- data.R
-- modules.R
-- module/
    -- module1.R
    -- module2.R

donyunardi avatar Aug 01 '22 23:08 donyunardi

Additionally, I think that it shouldn't be a separate package because users shouldn't be bothered to install a new package from github. I think we should rather keep it in teal.

Yes I agree

on creating a custom_modules folder, which I thought was a good idea

So my hope was that existing modules would soon be simplified enough to not need separate files for all of them (i.e. definitions.R -> which stores all the data extract specs etc. and then one file for the app). The custom_modules folder was originally meant to be used if your app used user-defined modules (i.e. a module created yourself not one from tmg, tmc etc.) but I'm open to whatever structure we think makes sense.

nikolas-burkoff avatar Aug 02 '22 12:08 nikolas-burkoff

So my hope was that existing modules would soon be simplified enough to not need separate files for all of them (i.e. definitions.R)

Sure, that sounds good too.

This skeleton template does seem like a possible solution for the marketplace. However, the template won't be able to show module visualization for the user to see before making a selection. The initial proposal was for the user to be able to see and select the module(s) from the marketplace. I am not sure if that requirement can be fulfilled using the skeleton template.

It feels like we're still undecided between the marketplace will produce one file vs multiple files. I think multiple files will improve code readability and maintenance, but this could lead to major changes to our vignettes. Would this be a good topic for refinement?

donyunardi avatar Aug 11 '22 00:08 donyunardi

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------------------------------
R/default_filter.R                   7       7  0.00%    17-27
R/dummy_functions.R                 74      61  17.57%   12-95
R/example_module.R                  17      17  0.00%    19-35
R/get_rcode_utils.R                 52       2  96.15%   94, 99
R/get_rcode.R                      131      51  61.07%   66, 73, 78, 139-148, 186, 212-261
R/include_css_js.R                  24       0  100.00%
R/init.R                            39      21  46.15%   171, 182-183, 236-257
R/log_app_usage.R                   38      38  0.00%    34-119
R/logging.R                         13      13  0.00%    11-28
R/module_nested_tabs.R             113      16  85.84%   57, 96, 100-117, 163, 212, 257
R/module_tabs_with_filters.R        65       0  100.00%
R/module_teal_with_splash.R         33       2  93.94%   62, 74
R/module_teal.R                    119      20  83.19%   49, 52, 142-143, 156-162, 168-174, 197, 227
R/modules_debugging.R               19      19  0.00%    41-60
R/modules.R                        115       9  92.17%   218, 423-448
R/project_skeleton.R                34      34  0.00%    13-60
R/reporter_previewer_module.R       12       2  83.33%   18, 22
R/show_rcode_modal.R                20      20  0.00%    17-38
R/utils.R                            6       0  100.00%
R/validations.R                     62      39  37.10%   103-355
R/zzz.R                             11       7  36.36%   3-14
TOTAL                             1004     378  62.35%

Results for commit: 94c2a219ad6be6efac788fd677cc29d1bedb52b3

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

github-actions[bot] avatar Aug 31 '22 10:08 github-actions[bot]

Unit Tests Summary

    1 files    10 suites   11s :stopwatch: 107 tests 107 :heavy_check_mark: 0 :zzz: 0 :x: 207 runs  207 :heavy_check_mark: 0 :zzz: 0 :x:

Results for commit b66d22fa.

github-actions[bot] avatar Aug 31 '22 10:08 github-actions[bot]

Oh god, it's happening

kpagacz avatar Sep 19 '22 07:09 kpagacz

Came across this so thought I'd share my 2 cents. It may have already been done but I'd highly suggest looking at golem and rhino because app templates are a big part of both. They also take very different approaches so it could be useful to compare and contrast.

asbates avatar Dec 15 '22 01:12 asbates

@asbates please see linked tasks to https://github.com/insightsengineering/NEST-roadmap/issues/9

pawelru avatar Dec 15 '22 08:12 pawelru