Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug or wrong use of panderOptions? #233

Closed
dmenne opened this issue Dec 7, 2015 · 11 comments
Closed

Bug or wrong use of panderOptions? #233

dmenne opened this issue Dec 7, 2015 · 11 comments
Assignees

Comments

@dmenne
Copy link

dmenne commented Dec 7, 2015

Using current version of pander from github with the panderOptions line gives an error; everything is fine without.


---
title: "Pandoc FTable"
author: "DM"
date: "7. Dezember 2015"
output: html_document

---

```{r setup, include=FALSE}
knitr::opts_chunk$set(echo = TRUE)
library(pander)
```

```{r}
panderOptions('table.alignment.default', function(df) 
  ifelse(sapply(df, is.numeric), 'right', 'left'))
pander(ftable(Titanic, row.vars = 1:3))
```
@daroczig
Copy link
Member

daroczig commented Dec 8, 2015

Thanks for reporting this issue. Unfortunately, I do not have a quick fix -- as this is a rather complicated problem, and not sure about the optimal solution yet. In short, the function passed to table.alignment.default should be able to return a character vector for each column, which is 6 in the above example:

----- ------ ----- -------- --- ---
                   Survived No  Yes

Class  Sex    Age                  

 1st   Male  Child           0   5 

             Adult          118 57 

      Female Child           0   1 

             Adult           4  140

 2nd   Male  Child           0  11 

             Adult          154 14 

      Female Child           0  13 

             Adult          13  80 

 3rd   Male  Child          35  13 

             Adult          387 75 

      Female Child          17  14 

             Adult          89  76 

Crew   Male  Child           0   0 

             Adult          670 192

      Female Child           0   0 

             Adult           3  20 
----- ------ ----- -------- --- ---

So 4 columns including strings and 2 columns including numbers, but the function defined above returns:

> ifelse(sapply(ftable(Titanic, row.vars = 1:3), is.numeric), 'right', 'left')
 [1] "right" "right" "right" "right" "right" "right" "right" "right" "right"
[10] "right" "right" "right" "right" "right" "right" "right" "right" "right"
[19] "right" "right" "right" "right" "right" "right" "right" "right" "right"
[28] "right" "right" "right" "right" "right"

So this is wrong usage of table.alignment.default, as the function should return a character vector with the length being 6 instead of 32 (which is the number of cells in the flat table).

But providing a more complex function to table.alignment.default (which returns c(rep('left', 4), rep('right', 2)) in this case) will not resolve the issue, as pander internally calls format on ftable objects before calling the user-defined alignment function, which converts everything to string -- so all columns would be aligned to left.

So two things to be done to resolve this issue:

  1. We need an improved function for table.alignment.default. I'd extend this primitive function by checking the class of the object, and returning length(attr(df, 'row.vars')) + length(attr(df, 'col.vars')) times left and maybe ncol(as.matrix(df)) times right for ftable objects or something similar.
  2. Find a way to avoid calling format before this custom function.

I will try to come up with a solution for (2), can you please work on (1)?

In the meanwhile, if you need an urgent solution and you do not have many tables to be printed automatically, you can manually set the justify param.

@dmenne
Copy link
Author

dmenne commented Dec 8, 2015

Thanks, daroczig, for your as always fast and detailed reply. I will check (1) later, but manual formatting would be totally ok for me. However, all the experimenting started when I could not get it to work manual formatting with ftable. Try (without any other options):

# works
pander(head(iris[,1:3], 2), justify = c('left', 'center', 'right'))
# nothing happens
pander(ftable(Titanic, row.vars = 1:3, 
        justify =c("left","center","right","left","center")))

This probably is due to (2),

For my actual report, using global "right" is ok, so this is no urgent at all. But I know you are a perfectionist.

@daroczig
Copy link
Member

daroczig commented Dec 9, 2015

I think you had a typo in the second example (not closing the parenthesis for the ftable call) and this seems to work OK:

> pander(ftable(Titanic, row.vars = 1:3), justify = c(rep('left', 4), rep('right', 2)))

----- ------ ----- -------- --- ---
                   Survived  No Yes

Class Sex    Age                   

1st   Male   Child            0   5

             Adult          118  57

      Female Child            0   1

             Adult            4 140

2nd   Male   Child            0  11

             Adult          154  14

      Female Child            0  13

             Adult           13  80

3rd   Male   Child           35  13

             Adult          387  75

      Female Child           17  14

             Adult           89  76

Crew  Male   Child            0   0

             Adult          670 192

      Female Child            0   0

             Adult            3  20
----- ------ ----- -------- --- ---

@dmenne
Copy link
Author

dmenne commented Dec 9, 2015

You were right, that error came up during experimenting. Here is what I see with your version:

---
title: "Pandoc FTable"
author: "DM"
date: "7. Dezember 2015"
output: 
  html_document:
    keep_md: true

---

```{r setup, include=FALSE, echo = FALSE}
knitr::opts_chunk$set(echo = TRUE)
library(pander)
```

```{r, echo = FALSE}
    pander(ftable(Titanic, row.vars = 1:3), 
        justify = c(rep('left', 4), rep('right', 2)))
```

2015-12-09 08_35_08-pandoc ftable

R version 3.2.2 (2015-08-14)
Platform: i386-w64-mingw32/i386 (32-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1

locale:
[1] LC_COLLATE=German_Germany.1252  LC_CTYPE=German_Germany.1252   
[3] LC_MONETARY=German_Germany.1252 LC_NUMERIC=C                   
[5] LC_TIME=German_Germany.1252    

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] pander_0.6.0

loaded via a namespace (and not attached):
[1] rsconnect_0.4.1.9 htmltools_0.2.7   tools_3.2.2       yaml_2.1.13      
[5] Rcpp_0.12.2       rmarkdown_0.8.1   knitr_1.11        digest_0.6.8     

@dmenne
Copy link
Author

dmenne commented Dec 9, 2015

Some added css better illustrates what happens. I will check md.
2015-12-09 08_42_28- _r_pandoc_pandocftable html

table tr:nth-child(odd) {
  background-color: #e6e6e6;
}

td {
  padding-left: 3px;
  padding-right: 3px;

}

@daroczig
Copy link
Member

daroczig commented Dec 9, 2015

The markdown looks OK to me, but the HTML/PDF rendered document is indeed off. I suspect something with the pandoc conversion, that will require some further debug. Very interesting problem, thanks again for reporting it!

@daroczig
Copy link
Member

daroczig commented Dec 9, 2015

As a quick workaround, you can use the rmarkdown style table, which works fine -- so I think this issue is related to no extra spaces around the "header" on the first line of the table. Will work on a fix. Note for myself: #186 and https://github.com/RomanTsegelskyi/pander/commit/d7da68674bab6151c8c110c1311f4fa03ffbe45b#diff-06d98d59ee432a4a652caa361f1bfda5R961

Quick demo on the workaround:

> pander(ftable(Titanic, row.vars = 1:3), justify = c(rep('left', 4), rep('right', 2)), style = 'rmarkdown')

|       |        |       |          |     |     |
|:------|:-------|:------|:---------|----:|----:|
|       |        |       | Survived |  No | Yes |
| Class | Sex    | Age   |          |     |     |
| 1st   | Male   | Child |          |   0 |   5 |
|       |        | Adult |          | 118 |  57 |
|       | Female | Child |          |   0 |   1 |
|       |        | Adult |          |   4 | 140 |
| 2nd   | Male   | Child |          |   0 |  11 |
|       |        | Adult |          | 154 |  14 |
|       | Female | Child |          |   0 |  13 |
|       |        | Adult |          |  13 |  80 |
| 3rd   | Male   | Child |          |  35 |  13 |
|       |        | Adult |          | 387 |  75 |
|       | Female | Child |          |  17 |  14 |
|       |        | Adult |          |  89 |  76 |
| Crew  | Male   | Child |          |   0 |   0 |
|       |        | Adult |          | 670 | 192 |
|       | Female | Child |          |   0 |   0 |
|       |        | Adult |          |   3 |  20 |

Renders as:

ftable1

@dmenne
Copy link
Author

dmenne commented Dec 9, 2015

Works for me. I always had assumed that rmarkdown was the default in pander when run in knitr.

@daroczig daroczig self-assigned this Dec 11, 2015
@daroczig
Copy link
Member

@dmenne I think it's fixed with the above commit, please let me know how it works.

@daroczig
Copy link
Member

Oh, and @RomanTsegelskyi, if you have some time, could you please have a look at the above commits and provide some feedback if you agree?

@dmenne
Copy link
Author

dmenne commented Dec 28, 2015

Fine for me (I had already forgotten, because your workaround worked also).

Thanks for your Christmas present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants