gforge/Gmisc — pull request #8 by mathematicalcoffee (2014-12-16 04:52:20+00:00)
Title: getDescriptionStatsBy with 2-level x with missing values

## The bug

This is rather specific (I only came across it while using getDescriptionStatsBy on my particular data), but when

* `x` is a 2-level factor with missing values
* `show_missing=FALSE`
* `add_total_col=TRUE`
* `show_all_values=FALSE` (the default)

Then `getDescriptionStatsBy` fails.

## Reproduce it

For example:

~~~
library(Gmisc)
# factors
extra = factor(sample(c('a', 'b', NA), nrow(iris), replace=T))
getDescriptionStatsBy(extra, by=iris$Species, show_missing=F, add_total_col=T, show_all_values=F)
~~~

returns error

~~~
Error in getDescriptionStatsBy(extra, by = iris$Species, show_missing = F, :
There is an discrepancy in the number of rows in the total table and the by results: 2 total vs 1 results
Rows total:a, b
Rows results:A
~~~

## The cause

When you drill down into it, it's the function `Gmisc:::prGetStatistics`.
To calculate the total column (since `add_total_col=T`), `getDescriptionStatsBy` calls `prGetStatistics` ([line reference](https://github.com/gforge/Gmisc/blob/master/R/getDescriptionStatsBy.R#L310)) on `x`.

In that function, there is a check to see if `x` has two levels present; if so, then depending on `show_all_values` one either presents the counts for all the values (`show_all_values=T`) or just the first one (`show_all_values=F`), along with missings.

`prGetStatistics` uses `length(unique(x)) == 2` as its 2-level check; in the case of a 2-level factor with NA values, `length(unique(x))` is 3 not 2, and so the code ignores the `show_missing`.
That is:

### expected

~~~
Gmisc:::prGetStatistics(factor(c(letters[1:2], NA)), show_all_values=F, show_missing=F)
a
1
~~~

### actual

~~~
Gmisc:::prGetStatistics(factor(c(letters[1:2], NA)), show_all_values=F, show_missing=F)
a b
1 1
~~~

The same thing happens when you have logical `x` (`c(T, F, NA)`) or character `x` with two distinct values as well as NA (`c('a', 'b', NA)`), due to the code handling 2-level logicals, characters and factors together.

## Fix

### 1

Omit NAs before checking for 2 unique levels. That is, change `length(unique(x)) == 2` to `length(unique(na.omit(x))) == 2`.

### 2

Also, the line where we extract just the entry of the table corresponding to the first factor uses

names(total_table) %in% c(levels(x)[1], "Missing")

In the case of logical or character `x`, `levels(x)` is `NULL`, so you get an empty table.
Assuming that you intend to deal with logical/character `x` in this function (assumed so because of the `is.factor(x) || is.logical(x) || is.character(x)` check in the function), one way to work around this is to convert `x` into a factor and take the first level.

That is, change `levels(x)[1]` to `levels(as.factor(x))[1]`.

Looking at the code for `as.factor`, it only does the conversion if `x` is not already a factor, so I don't think this should cost too much extra computation.

### 3

Finally, added a test for the 2-factor-with-NA case, but it's pretty sparse. It tests `prGetStatistics` directly rather than calling `getDescriptionStatsBy` and hoping for a non-error, since the bug is in `prGetStatistics`. If you rather test `getDescriptionStatsBy` (since `prGetStatistics` is private) I can rewrite the test.

## Notes

* now that we omit NA before checking for unique levels, factor/character/logical `x` that previously consisted of one distinct value plus NAs now fall down to the next case. This is fine because there's only one level to show anyway, so `show_missing` is moot here.
* I probably should add more tests for the logical/character case, and a test with the above (NA and one distinct value), but I'm no good at tests :( But I'll give it a shot if you want them and don't have time to write them.