Create wrappers for all possible Github/local functions

[wchodor]

No description provided.

[MarcinKosinski]

There is type parameter in createEmpyGithub so you should also remove
it there :)
But this parameter has been already published on CRAN.

2015-12-08 13:48 GMT+01:00 Witold Chodor notifications@github.com:

I would like to take care of this mission but I have some doubts whether
you agree on that. I suggested a solution here(#149
#149) to omit using type
parameter.
@pbiecek https://github.com/pbiecek @MarcinKosinski
https://github.com/MarcinKosinski
What do you think about that?

loadFromRepo <- function( md5hash, repoDir = NULL,
repo = NULL, user = NULL, branch = "master", repoDirGit = FALSE,
value = FALSE ){

local <- (!is.null(aoptions("repoDir")) && is.null(repo)) || (!is.null(repoDir) && is.null(repo))
GitHub <- (is.null(repoDir) && !is.null(aoptions("repo"))) || (is.null(repoDir) && !is.null(repo))
if (local) {
loadFromLocalRepo( md5hash = md5hash, repoDir = repoDir, value = value )
} else if (GitHub) {
loadFromGithubRepo( md5hash = md5hash, repo = repo, user = user, branch = branch,
repoDirGit = repoDirGit, value = value )
} else {
stop("repo and repoDir parameters can not be used simultaneously.")
}
}

—
Reply to this email directly or view it on GitHub
#153.

[MarcinKosinski]

@wchodor Ok let's create wrappers with the functionality as in loadFromRepo and leave createEmptyGithubRepo for now, but at the end of this task pleas create a new one with assignmnent to me to decide on how to change local/type-parameter convention in createEmptyGithubRepo

[wchodor]

We have a lot of work to do ahead of us :)

[MarcinKosinski]

Due to new behavior of loadFromRepo wrapper we are having 3 fails in tests:

Testing archivist
Directory test1111 did not exist. Forced to create a new directory..Directory tmp_archivist did not exist. Forced to create a new directory.........12Directory tmp_archivist did not exist. Forced to create a new directory...Directory new_repo did not exist. Forced to create a new directory........3....Directory /tmp/RtmpDuqttx/file59b57a8bc9a did not exist. Forced to create a new directory.......Directory repository did not exist. Forced to create a new directory..Directory repository did not exist. Forced to create a new directory..Directory repository did not exist. Forced to create a new directory...Directory test1234 did not exist. Forced to create a new directory.  adding: test1234/backpack.db (deflated 87%)
  adding: test1234/gallery/ff575c261c949d073b2895b05d1097c3.rda (deflated 71%)
  adding: test1234/gallery/ff575c261c949d073b2895b05d1097c3.txt (deflated 70%)
..
1. Error: aread downloads files ---------------------------------------------------------------------------------
repoDir and repo or user or branch or repoDirGit were used simultaneously. Remember
to use them separately!
1: withCallingHandlers(eval(code, new_test_environment), error = capture_calls, message = function(c) invokeRestart("muffleMessage"))
2: eval(code, new_test_environment)
3: eval(expr, envir, enclos)
4: aread("pbiecek/graphGallery/2166dfbd3a7a68a91a2f8e6df1a44111") at test_jss_artilce.R:2
5: loadFromRepo(md5hash = elements[length(elements)], repo = elements[2], repoDirGit = ifelse(length(elements) > 3, paste(elements[3:(length(elements) - 
       1)], collapse = "/"), FALSE), user = elements[1], value = TRUE) at /home/mkosinski/alink/archivist/R/aread.R:56
6: stop("repoDir and repo or user or branch or repoDirGit were used simultaneously. Remember\nto use them separately!") at /home/mkosinski/alink/archivist/R/loadFromRepo.R:370

2. Error: asearch works properly --------------------------------------------------------------------------------
repoDir and repo or user or branch or repoDirGit were used simultaneously. Remember
to use them separately!
1: withCallingHandlers(eval(code, new_test_environment), error = capture_calls, message = function(c) invokeRestart("muffleMessage"))
2: eval(code, new_test_environment)
3: eval(expr, envir, enclos)
4: asearch("pbiecek/graphGallery", patterns = c("class:lm", "coefname:Sepal.Length")) at test_jss_artilce.R:26
5: lapply(paste0(repo, "/", oblist), aread) at /home/mkosinski/alink/archivist/R/asearch.R:132
6: FUN(X[[i]], ...)
7: loadFromRepo(md5hash = elements[length(elements)], repo = elements[2], repoDirGit = ifelse(length(elements) > 3, paste(elements[3:(length(elements) - 
       1)], collapse = "/"), FALSE), user = elements[1], value = TRUE) at /home/mkosinski/alink/archivist/R/aread.R:56
8: stop("repoDir and repo or user or branch or repoDirGit were used simultaneously. Remember\nto use them separately!") at /home/mkosinski/alink/archivist/R/loadFromRepo.R:370

3. Error: object is properly serialized -------------------------------------------------------------------------
repoDir and repo or user or branch or repoDirGit were used simultaneously. Remember
to use them separately!
1: withCallingHandlers(eval(code, new_test_environment), error = capture_calls, message = function(c) invokeRestart("muffleMessage"))
2: eval(code, new_test_environment)
3: eval(expr, envir, enclos)
4: aread("pbiecek/graphGallery/2a6e492cb6982f230e48cf46023e2e4f") at test_jss_artilce.R:104
5: loadFromRepo(md5hash = elements[length(elements)], repo = elements[2], repoDirGit = ifelse(length(elements) > 3, paste(elements[3:(length(elements) - 
       1)], collapse = "/"), FALSE), user = elements[1], value = TRUE) at /home/mkosinski/alink/archivist/R/aread.R:56
6: stop("repoDir and repo or user or branch or repoDirGit were used simultaneously. Remember\nto use them separately!") at /home/mkosinski/alink/archivist/R/loadFromRepo.R:370

Could you pleas somehow rewrtie aread and asearch that they would not use loadFromRepo in the current implementation that is not consistant with our tests and previous versions of archivists? After tests could you pleas check whether the examples submitted to JSS are working?
https://github.com/pbiecek/archivist/blob/master/scripts/replicationScript.Rmd

[MarcinKosinski]

@pbiecek @wchodor Coming back to writing new functionalities/function that will work on object of class repository (that strictly distinguish between Local and GitHub modes):

Since we are going to write wrappers, and are about to change archivist philosophy a little bit, what would you say about writing such wrappers that would harmonize and brighten the archivist functionality:

Functions that use Tags

• Tags(repository, method = 'add') -> addTagsRepo
• Tags(repository, method = 'get') -> getTagsRepo
• Tags(repository, method = 'split') -> splitTagsRepo
• Tags(repository, method = 'search/multisearch') -> search/multisearch TagsRepo

Functions that use Repository

• Repo(repository, method = "show", ...) -> showRepo
• Repo(repository, method = "summary", ...) -> summaryRepo
• Repo(repository, method = "zip", ...) -> zipRepo
• Repo(repository, method = "set", ...) -> setRepo
• Repo(repository, method = "delete", ...) -> deleteRepo
• Repo(repository, method = "clone", ...) -> cloneRepo
• Repo(repository, method = "copy", ...) -> copyRepo
• Repo(repository, method = "shinySearch", ...) -> shinySearchRepo
• Repo(repository, method = "create") -> createRepo
• Repo(repository, method = "parametrize") -> parametrizeRepo
• Repo(repository, method = "pull") -> pullRepo
• Repo(repository, method = "push") -> pushRepo

Functions that use md5hashes

• md5hash(repository, md5hashes, method = "load") -> loadFromRepo
• md5hash(repository, md5hashes, method = "remove") -> rmFromRepo
• md5hash(repository, md5hashes, method = "replace") -> replaceMD5HAS_with_another_one

We would have less documentation pages, all functionalities that works similiar will be in the same place and the vision ans usecases (and maybe tutorials) would be more compact.

We could then leave functioalities for artifacts as they are now:

• alink
• aread
• asearch
• ahistory

and we would have the main function archive that will work the same as saveToRepo right now, and eventually it will saves objects to GitHub repositories.

What do you think about that?

Note:

parametrizeRepo is implemented here: https://github.com/pbiecek/archivist/blob/master/devel/parametrizeRepo.R

[MarcinKosinski]

Or we can go further and create repository as S4 class and create methods for him:

Repo@show(...) -> showRepo
Repo@summary(...) -> summaryRepo
Repo@set(...) -> setRepo

Repo@Tags@add()
Repo@Tags@get()
Repo@Tags@split()
Repo@Tags@search

Repo@md5hash@load
Repo@md5hash@remove
Repo@md5hash@replace

[MarcinKosinski]

I think creating wrappers around all existing function qould require creating a new object's class which for practical reason should be of class S4 and could work as here #153 (comment) .
I think we should close this issue or put away for the future but in a a new package : archivist.S4 os something similiar. For now, loadFromRepo #182 used in aread should be reimplemented or removed from a package.

[pbiecek]

Ok, I will do this with #189