Fixed tables - Githubissues

SimonGoring / neotoma_paper

Repository for the neotoma package paper.

MIT License

3 stars 2 forks source link

Fixed tables #23

Closed karthik closed 10 years ago

karthik commented 10 years ago

This commit will fix:

Tables. This requires that you install the pander package.
It also addresses the issue with unreadable code blocks in #22. For readability, especially for novices, I removed the excessive nesting and split up the code a little bit. I also added tidy = FALSE to that chunk to override knitr/formatR from reflowing it.

gavinsimpson commented 10 years ago

Re the extra tidy = FALSE - OK, got ya, think I've experienced that before too. Let's clean out all the extra tidy = FALSE statements as part of this PR and then we can troubleshoot if chunks aren't respecting this, which may be a bug in knitr.

gavinsimpson commented 10 years ago

This is great @karthik - lots of tedious fixes to improve consistency are not the most glamorous of contributions but the paper is all the better for it. The tables look nice too and lone map a little less garish.

Only major comment is when did we include the knitr cache in the repo? Seems that this is going to enlarge all our commits from now on.

karthik commented 10 years ago

Thanks for the review @gavinsimpson I had a chat with Simon today and this seemed like an issue that needed to get resolved anyway. I'll fix the issues you pointed out before this gets merged.

Good point about the cache, I may have accidentally committed it. I'll also remove it tomorrow.

karthik commented 10 years ago

@gavinsimpson Fixing stuff you pointed out now but I think I may have figured out the bug with knitr. I'll see if there is already a PR at yihui's repo otherwise I'll file one there.

karthik commented 10 years ago

I believe I have addressed all the issues above. Please merge if it looks ok or let me know if I missed anything.

SimonGoring commented 10 years ago

I can merge this, but @gavinsimpson, where you're suggesting changes do I need to merge, then change, then commit again?

gavinsimpson commented 10 years ago

@SimonGoring Wait on merging this - don't think we want the cache in the repo. There are also a few other minor things that @karthik could handle here related to the changes he's made.

gavinsimpson commented 10 years ago

@karthik Thanks for tackling all those comments. Two main requests for changes before this is merged:

The cache objects are still in the PR. Don't think we want those in the repo.
Your editor has inserted tabs when indenting. Can we convert all those tabs to spaces? That is why the github renderer is having trouble aligning the code.

gavinsimpson commented 10 years ago

That's great @karthik . Let's get this one merged now so we can see what is left to do and where further cleanup or tidying of the code is needed - I've got a few things I want to tweak in the code to streamline it.

:+1: for merge from me.