CrumpLab / vertical

R workflow for sharing psychological research
https://crumplab.github.io/vertical/
Other
35 stars 2 forks source link

Simplify build; one function to rule them all #41

Closed mvuorre closed 4 years ago

mvuorre commented 4 years ago

I could not get the CLI init_vertical_project to work with arguments. This prompted me to take another look at the build processes, and refactoring it all into one function that works both in GUI and CLI, implemented in this PR.

The only remaining problem is that sometimes (yes, sometimes; I can't figure out what influences this) RStudio opens two new windows when GUI is used to create the new project.

CrumpLab commented 4 years ago

Quick question. I briefly looked at the new function. Does this eliminate the functionality from init_vertical_project() that allows a user to interactively create a project? e.g., by answering questions etc.

mvuorre commented 4 years ago

Yes, my bad, sorry! I am not sure if it is that helpful because you can do it interactively through the gui. But you can add it back in if wanted.

Also, I could not use init_vertical_project() in an existing folder. It resulted in errors when copying the files, either into a completely new empty folder or one with files already. I therefore removed that functionality after further thought. We can add it in though, but it needs to be created in a different way.

CrumpLab commented 4 years ago

No problem, this is good to get on the same page for this.

Let me test your solution for a bit. I've done a bit of testing and found that some weird things can happen if you only supply a name and no path. It gives a warning, but will continue to do things that a user probably doesn't want to happen (e.g., build a vertical project as a folder inside a current folder).

CrumpLab commented 4 years ago

There's an issue with EDIT: this issue occurs from CLI if a full path is specified.

if (dots$init_som) init_supplemental(prjct_name = path)

path contains the full path, so when later:

usethis::use_template(template = "article.Rmd",
                          save_as = "vignettes/Supplementary_1.Rmd",
                          data = list(vignette_title="Supplementary analyses",
                                      Package = prjct_name),
                          package = "usethis")

The vignette populates a field in the library() call with the whole path, rather than the name of the package. Let me see about fixing this.

EDIT: should be able to fix this with basename() to extract the final foldername

if (dots$init_som) init_supplemental(prjct_name = basename(path))
mvuorre commented 4 years ago

Yes I saw that just now as well, I don't think any of the init_ functions should take any arguments. There was something odd with using just usethis::use_article() but I think just having that in there is the best way to go

CrumpLab commented 4 years ago

I think the issue with usethis::use_article() was the weird ghosted file issue. That function automatically opens the file in the editor, and that can't be turned off. Using use_template() solved that issue, but required the input parameter

CrumpLab commented 4 years ago

I've done the merge. I would like to make some additions, but I like this simplifying change and it makes sense to modify further from this point