-
-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Example skeleton project - DO NOT MERGE #701
base: main
Are you sure you want to change the base?
Conversation
This looks great, @nikolas-burkoff!
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. |
Icon: teal.png | ||
|
||
Parameter: use_cdisc | ||
Widget: CheckboxInput |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(in the future) Can the Widget be for example?
Widget: availableModulesInput
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can only have the following:
We may want an addin to do the add modules part rather than at this project creation time. And for creating your own custom modules you could maybe? use snippets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is snippet a Rstudio-exclusive feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think snippet is RStudio exclusive feature (i.e. you can add code snippet on vscode too). However, the syntax will different.
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 I like addon because it promotes good practice of using "New Project" (at least from RStudio). |
@kumamiao @donyunardi we have not decided exactly what is needed here. If addin should be a single file or multiple files. 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. |
@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 What do you think if we separate the module call into its own file under a separate folder? app.R
data.R
modules.R
module/module1.R
module/module_2.R
The structure would look like this:
|
Yes I agree
So my hope was that existing modules would soon be simplified enough to not need separate files for all of them (i.e. |
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? |
Code Coverage Summary
Results for commit: 94c2a21 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Oh god, it's happening |
@asbates please see linked tasks to insightsengineering/NEST-roadmap#9 |
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 insightsengineering/NEST-roadmap#59
We now have:
Or if you chose not to have cdisc:
Clearly if we are doing this for real then
package_skeleton
function can be called from other IDEspackage_skeleton
function in a nicer way and output a proper structure not just a single app.R fileYou could even have options to create specific apps i.e. see insightsengineering/NEST-roadmap#9 (comment) - although they wouldn't live in teal itself though could live in teal.gallery @dinakar29