Refactor code #4
Conversation
…ions to get_bg and get_grid functions
R/doc.R
Outdated
| ##' @author Toby Dylan Hocking | ||
| ##' @export | ||
| ##' | ||
| makeDocs <- function(doc.dir){ |
There was a problem hiding this comment.
I would suggest deleting this in animint2
|
I would suggest removing the |
|
I too think removing the |
|
@tdhock Do we need to retain the |
|
it would be great to remove the gg2animint function |
R/animint.R
Outdated
| @@ -955,7 +950,6 @@ animint2dir <- function(plot.list, out.dir = tempfile(), | |||
| g.list <- list() | |||
| for(p.name in names(ggplot.list)){ | |||
| ggplot.info <- ggplot.list[[p.name]] | |||
| meta$prev.class <- NULL # first geom of any plot should not be next. | |||
| for(layer.i in seq_along(ggplot.info$ggplot$layers)){ | |||
There was a problem hiding this comment.
Do not need this, I think. It is not updated anywhere in the code
|
@tdhock I was thinking we could merge this PR now, and work on the rest of clean up later. Some part of the code is a bit tricky to deal with, especially selectors that are passed through the |
|
ok
…On Fri, Jun 9, 2017 at 11:11 AM, Faizan Uddin Fahad Khan < ***@***.***> wrote:
@tdhock <https://github.com/tdhock> I was thinking we could merge this PR
now, and work on the rest of clean up later. Some part of the code is a bit
tricky to deal with, especially selectors that are passed through the meta
environment. We can sort it out later.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA478n4FkqDYd6BQKOaDyp_qggBr_szwks5sCWC5gaJpZM4NhSfd>
.
|
|
Just as a last note, what should we keep the version number? |
|
no need for v2 in front of the version number, but please change the
package name to animint2.
I would recommend to continue using dates for version numbers, e.g.
2017.06.12
…On Sun, Jun 11, 2017 at 8:41 PM, Faizan Uddin Fahad Khan < ***@***.***> wrote:
Just as a last note, what should we keep the version number? v2.2017.06.12
?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AA478p51HiDUno3hARuMArfDCpvE2UhIks5sDIlGgaJpZM4NhSfd>
.
|
I think the current code in the
animintcompiler is a bit hard to debug. Especially because the functions are too long (saveLayerspans about 500 lines) and the implementation is mixed up with the logic. As such I sometimes find it difficult to locate the problem as I have to wade through a lot of unrelated code before I get to the buggy part. And often I have a lot of variables that were probably used a 100 lines back cluttering the work-space.This PR has 2 motives:
Separate the implementation from the code: The implementation is separated in a file
animintHelpers.R(for now), while the logic stays in the top level functions. This will avoid us going through trivial code in a debugger that probably is totally unrelated to the problem.Reduce the size of functions to easily spot errors and reduce the number of active variables
So the code in the top level functions looks like this:
https://github.com/faizan-khan-iit/animint2/blob/refactor/R/animint.R#L153-L164
with their implementations in other files.
Note: The file
helperFunctions.Ris composed of functions we probably will never need to care about. So I separated those too.