KentonWhite / ProjectTemplate

A template utility for R projects that provides a skeletal project.
http://projecttemplate.net
GNU General Public License v3.0
622 stars 159 forks source link

Consider removing "cache" from .mandatory.files #273

Closed jeromyanglim closed 6 years ago

jeromyanglim commented 6 years ago

Report an Issue / Request a Feature

I'm submitting a (Check one with "x") :


Issue Severity Classification -

(Check one with "x") :

Expected Behavior

If you choose not to cache data, then you do not need to have a cache directory. Thus, I have sometimes not included that directory in my projects.

ProjectTemplate did not previously require a cache folder. Thus, any projects that excluded this folder now no longer run when you run load.project(). Instead, they give the rather cryptic error message:

Current Directory: ... is not a ProjectTemplate directory
Please change to correct directory and re-run load.project()

So for example, I have quite a few tutorials, exercises, data analysis projects and so on that are archived in various places. New users to ProjectTemplate will be picking these up and possibly wondering why the project is not working when they are seemingly in the right working directory.

Obviously the simple solution for these people (should they land here through google) is simply to add a directory called "cache".

Current Behavior

I've just upgraded to 0.8.2 of ProjectTemplate and now it tells me that this project is not a ProjectTemplate directory when I don't have a cache folder.

ProjectTemplate:::.mandatory.files
[1] "config/global.dcf" "cache"             "data"    
Possible Solution

The simple solution would be to remove "cache" from the vector .mandatory.files.

This is not an essential file for ProjectTemplate.

Another option would be to tweak the error message to indicate that a mandatory file is missing.

That said, I can understand that if you leave it as is, new projects will not assume you can exclude "cache". So it will sort itself out going forward.

Hugovdberg commented 6 years ago

I agree that cache isn't necessary if caching is disabled in the config. On the other hand the logic becomes quite a lot more complicated if we make the mandatory files dependent on the current config. Isn't this fixed for you by running migrate.project on the current project? I guess you also get a warning your project configuration is outdated since this change to mandatory files was already introduced two years ago.

jeromyanglim commented 6 years ago

You don't get the warning about updating the config version.

For example, I took an existing project that had a version 0.6 config file and deleted the cache directory. You get the following message:

> library(ProjectTemplate); load.project()
Current Directory: finished-analysis is not a ProjectTemplate directory
Please change to correct directory and re-run load.project()

........

As an aside, if you've noticed my flurry of questions. I just updated my laptop so presumably, I've been running an older version of ProjectTemplate for quite a while. That's why I'm just now noticing these backwards compatibility challenges.

KentonWhite commented 6 years ago

I think adding the cache file should be handled in the migrate script. When running migrate.project if a mandatory file is missing, a warning should be given and the mandatory file added. Thoughts?

jeromyanglim commented 6 years ago

That makes sense to me given that it is now considered a required file.