mlr-archive / mlr-tutorial

The mlr package online tutorial
http://mlr-org.github.io/mlr/
20 stars 11 forks source link

Add code to bottom of each tutorial #111

Closed SteveBronder closed 6 years ago

SteveBronder commented 7 years ago

Creates a new function in ./build called purlIt that takes the code from each tutorial and writes it to the folder full_code_src. This code is then printed at the bottom of each tutorial with (ignore the slash)

## Full Code:
```{r, purl=FALSE, echo = FALSE, comment=NA}
cat(readLines('full_code_src/advanced_tune.R'),fill = 2)
\```

CHANGED FILES:

masongallo commented 7 years ago

Very cool, I like it

Edit: you almost gave me a heart attack when I saw the 20k LOC change

SteveBronder commented 7 years ago

you almost gave me a heart attack when I saw the 20k LOC change

You and me both.

The actual addition is one function in the ./build though

jakob-r commented 7 years ago

That's some nice work already!

However, I am pretty sure we don't have to edit the Rmd file manually. Look at the build file:

lines = str_replace_all(lines, macro$pattern, macro$replacement)

There we already apply the macros on all lines. If it is possible we should have a markup meta information at the beginning of the Rmd file like

---
full_code: true
---

If this meta information is set to true we can just append some lines (pseudo code:)

base.name = getFileBaseName(???)
lines = c(lines,
"## Full Code:",
"```{r, purl=FALSE, echo = FALSE, comment=NA}",
sprintf("cat(readLines('full_code_src/%s.R'),fill = 2)",base.name),
"```")
jakob-r commented 7 years ago

And I think you should take the html, zip and pdf files out of the commit as they will be generated by travis anyhow.

SteveBronder commented 7 years ago

Just to make sure I am understanding.

Remove the zip file and html from devel along with the pdf folder and pdf file?

EDIT: I just did a merge so everything is up to date with gh-pages

SteveBronder commented 7 years ago

If it is possible we should have a markup meta information at the beginning of the Rmd file like

How would I go about checking for this? I'm rather inexperienced in parsing markup meta-info. Also this would require me to go through each tutorial and place that at the top, which I'm not sure how to automate.

While my approach is less clever I feel like it's not a huge burden to tell tutorial writers, "To place the full code at the bottom of your tutorial add

## Full Code:
```{r, purl=FALSE, echo = FALSE, comment=NA}
cat(readLines('full_code_src/your_file_name.R'),fill = 2)
\```

"

SteveBronder commented 7 years ago

Current build is passing!

schiffner commented 7 years ago

Hi,

first of all thx very much, Steve.

I'm siding with Jakob in that I think that we should not add the full code to the .Rmd files by hand. In my experience every time I thought that sth. wasn't going to be hard to maintain manually I learned better shortly after. For example if we decide to also add links to the R-code on each tutorial page we need to touch everything again. If knitr options change or we want to change chunk options etc. Also sorry for nitpicking, but the header should be ## Full code rather than ## Full Code:.

I don't think that we need markup information in every .Rmd-file either (since as you, Steve pointed out that would also mean to touch each file if sth. needs to be changed). In the meantime we could collect this info in the build file and later have a central config file for this.

You are adding the full code to each tutorial page anyway at the moment, correct? Just a question: Did you build it locally and did you check the resulting html? I'm asking because I'm not sure what will happen to some of the pages in "Extend" because they are using chunks with the code option.

Anyway, I would follow Jakob's suggestion and in the purlIt function append the lines necessary to show the full code to each .Rmd-file.

SteveBronder commented 7 years ago

I'm siding with Jakob in that I think that we should not add the full code to the .Rmd files by hand. In my experience every time I thought that sth. wasn't going to be hard to maintain manually I learned better shortly after.

I trust your judgement!

You are adding the full code to each tutorial page anyway at the moment, correct? ... Anyway, I would follow Jakob's suggestion and in the purlIt function append the lines necessary to show the full code to each .Rmd-file.

Yes going off the advice of you and Jakob I am placing this in the knitIt() function

  lines = c(lines, "## Full code",
    "```{r, purl=FALSE, echo = FALSE, comment=NA}",
    paste0("cat(readLines('./",file.path("full_code_src", str_replace(f, "\\.Rmd$", "\\.R")), "'),fill = 2)"),

It does not manipulate the files themselves and instead just adds the code to the end of lines. Though a bit ugly, it does the job.

Just a question: Did you build it locally and did you check the resulting html? I'm asking because I'm not sure what will happen to some of the pages in "Extend" because they are using chunks with the code option.

So there is a problem when chunks use the code option. See here in create_imputation the chunk with the code flag produces an NA. What's the best way to fix this?

SteveBronder commented 7 years ago

How much do we use the code flag? For each chunk that uses it we could also set purl = FALSE

schiffner commented 7 years ago

Not much. From the top of my head just in create_impuation and create_learner.

EDIT: And using purl = TRUE would solve it? I.e. the code would render correctly in the respective code chunk and the bottom?

SteveBronder commented 7 years ago

Why is this saying it was closed and merged? I'm so sorry if I accidentally merged this, I did not click anything, I must have done something by accident??

EDIT: I think everything is fine and the closing was caused by me reverting back to what is currently on the gh-pages branch. I commited the changes and everything is fine now

SteveBronder commented 7 years ago

But anyway, the ## NA was actually being cause by comment = NA in the full code block at the end. If it saw a blank chunk it would just do ## NA. That has now been changed to comment= '' so the NA's do not show up. Also we are not purling the showFunctionDef function in create_imputation and create_learner because that is internal

SteveBronder commented 7 years ago

bump

jakob-r commented 7 years ago

I edited it so you can write the following in the YAML front matter:

---
params:
  full.code: FALSE
---
# Machine Learning in R: mlr Tutorial
...
jakob-r commented 7 years ago

I also removed the full code chapters from the pdf

SteveBronder commented 7 years ago

Excellent! Is this ready to merge?

jakob-r commented 7 years ago

Not completely. If you have time please build it locally and check all pages wheter the "complete code" makes sense or not.

SteveBronder commented 7 years ago

Sure. Just started my new job, but I should have time this weekend

SteveBronder commented 7 years ago

Looking this over it looks good! I had to remove the purl option from two of the chunks in create_imputation_learner.rmd file so we did not get back ## NA comments in the bottom code.

SteveBronder commented 7 years ago

Travis is getting this back

The command "tlmgr update --self" failed and exited with 1 during .

odd? I think it's happening across the different branches

jakob-r commented 7 years ago

Thanks for looking through this. tlmgr is the latex packet manager. I will check what's going on there.

SteveBronder commented 7 years ago

Weird, getting this error now?

! LaTeX Error: File `xkeyval.sty' not found.
Type X to quit or <RETURN> to proceed,
or enter new name. (Default extension: sty)
Enter file name: 
! Emergency stop.
<read *> 

l.67 \eu@ifbooltrue
pandoc: Error producing PDF from TeX source
Error in system3("./build-pdf.py") : 
  Command: ./build-pdf.py ; exit code: 1; output: NA
Calls: system3 -> stopf
Execution halted
jakob-r commented 7 years ago

Yup. Also happening in other checks.

SteveBronder commented 7 years ago

@jakob-r do you know why the tests are hanging when it tries to knit partial_dependence.Rmd?

jakob-r commented 7 years ago

I remember that I looked into it and saw that the partial dependence stuff at one point got considerably slower in the mlr Travis checks but than I noticed that nearly all tests got slower. I then thought there was some change in mlr which somehow drastically lowered the performance but than some days later everything went back to normal for mlr Travis leading me to the thought that maybe just Travis had some performance problems. That's why I will just restart Travis here and hope it will get faster.

jakob-r commented 7 years ago

Now we are just hitting the time limit.

SteveBronder commented 6 years ago

:pray: Having travis again :pray:

Tests pass. @jakob-r want to give this a quick look through and then I'll squash and merge?

jakob-r commented 6 years ago

Thanks for your patience. I made some adaptions to the newest mkdocs version. Let's see if it works on travis.

jakob-r commented 6 years ago

Currently we have a time out. We could reduce the running time significantly by using the same cache for the pdf and the html version. See #64

jakob-r commented 6 years ago

Thanks again @SteveBronder

SteveBronder commented 6 years ago

Hey actually we should revert this. Seeing

full.code: FALSE at the top of some of the pages

https://mlr-org.github.io/mlr-tutorial/devel/html/

SteveBronder commented 6 years ago

Or @jakob-r how do we hide that? Could just add a commit and remerge

jakob-r commented 6 years ago

whoops. did not see this. will fix it