eric-pedersen / mixed-effect-gams

Repo for tutorial/paper on mixed-effect GAMs
MIT License
122 stars 32 forks source link

Final preflight checks #46

Closed gavinsimpson closed 5 years ago

gavinsimpson commented 5 years ago

Issue for anything we want to do before we resubmit, but which isn't a trivial edit to fix a typo. This is an incomplete list based on what I have noticed from a scan through yesterday; please add more things here if you notice them so we don't forget.

eric-pedersen commented 5 years ago

Thanks for putting this together, Gavin! I just updated the to-do list with going through the response to reviewers. I started working on that yesterday as a Google Doc, so hopefully will have it together soon.

eric-pedersen commented 5 years ago

I've altered Table 2 as suggested, but I couldn't figure out how to get a prime symbol to render in the table; it kept rendering as a backwards single quotation mark. I think for now it's okay to leave it as is, as that is how the term appears in mgcv, and it shouldn't cause too much confusion in the table itself.

eric-pedersen commented 5 years ago

I've gone through and I think implemented all of these changes. @gavinsimpson, want to check to make sure you're good with all the tweaks? Especially table 2; I wasn't able to get the k' working properly in the table, but maybe someone with better Rmarkdown-fu can figure it out.

I decided to go with a fully different colour scheme for figure 2 in commit 3176e64, as the discrete viridis scale is too difficult to distinguish without the yellow end. I found a good discrete scale, with clear visual separation of categories, in ggthemes, and have implemented that. What do you think of this new scheme?

capture

Also, I've gone through the code again to see any places that could be cleaned up, but this could use a second or third pair of eyes.

gavinsimpson commented 5 years ago

That looks great @eric-pedersen

I can take a look at the first one when I do a build a read through this PM. I'm just trying to fix a little bug in gratia which is leading to inconsistent results and is stopping me submitting to CRAN.

eric-pedersen commented 5 years ago

I've fixed this now in the most recent commit, and updated the figure above to show the new alignment (also: plot_grid is such a neat function!). Does the updated figure look good to you?

gavinsimpson commented 5 years ago

I've altered Table 2 as suggested, but I couldn't figure out how to get a prime symbol to render in the table; it kept rendering as a backwards single quotation mark. I think for now it's okay to leave it as is, as that is how the term appears in mgcv, and it shouldn't cause too much confusion in the table itself.

I tried to take a look and it seems that there is some incompatibility with how we are rendering the tables, injecting latex into the columns names, and our rendering format. We need to have escape = FALSE for the table to get the latex rendering but if we do that, now matter how I fiddled with things I couldn't get texi2dvi to complete the build of the PDF without erroring out. So agreed; leave this as is now.