Rethink coerce_factors, fixes 163

[ewallace]

Rethinks coerce_factors, fixes #163

• replaces warning by message when coerce_factors is used for well_col and well_row
• replaces warning by message for target_id and sample_id in label_plate_rowcol
• asserts necessary column names in display_plate and calculate_... functions, fixes #102

Overall this should make some of the common functions more user-friendly.

[DimmestP]

I believe the changes do address the issue. However, in checking these new changes I found a bug that must have existed previously.

When we coerce colkey$well_col and rowkey$well_row to be factors we extract the levels from plate$well_col and plate$well_row. This assumes plate$well_col and plate$well_row are factors themselves, but we don't actually check that. If plate$well_row is actually a character vector then the join fails silently.

example_plate =  tibble(well_row=LETTERS[2:7], well_col=2:7)

example_rowkey = tibble(well_row=LETTERS[2:7], target_id = "Z")

label_plate_rowcol(plate = example_plate, rowkey = example_rowkey)

# A tibble: 6 × 3
>  well_row well_col target_id
>  <chr>       <int> <chr>    
> 1 B               2 NA       
> 2 C               3 NA       
> 3 D               4 NA       
> 4 E               5 NA       
> 5 F               6 NA       
> 6 G               7 NA 

Should we assert that plate$well_row must be a factor for the function to run or should we coerce it to a factor too?

[DimmestP]

I have fixed this bug by coercing plate$well_col and plate$well_row to factors if coercefactors = TRUE. However, if this happens a warning is outputted.

Warning messages: 1: In label_plate_rowcol(plate = example_plate, rowkey = example_rowkey) : plate$well_col is not a factor. Automatically generating plate$well_col factor levels. May lead to incorrect plate plans. 2: In label_plate_rowcol(plate = example_plate, rowkey = example_rowkey) : plate$well_row is not a factor. Automatically generating plate$well_row factor levels. May lead to incorrect plate plans.

I think it should be a warning not a message as it goes against the standard usage of the function. I've also added an explanation in the function manual.

If plate$well_col or plate$well_row are not factors and coercefactors = TRUE label_plate_rowcol will automatically convert them to factors, but will output a warning telling users this may lead to unexpected behaviour.

If you are happy @ewallace I'll merge.

[ewallace]

This is a really nice bug fix and more defensive programming.

I'm just wondering if we should replace those calls to factor with forcats::as_factor which has somewhat more predictable behaviour? https://forcats.tidyverse.org/reference/as_factor.html

[ewallace]

• changed a few calls to factor to calls to as_factor.
• that simplified some of the create_plate... functions, e.g. as_factor(c(rep("+RT", 18), rep("-RT", 6))) automatically has factors in the right order so don't need to explicitly specify levels.
• replaced all stopifnot calls with assert_that for better behaviour and error messages.

Overall I'm really happy this pull request has improved the plate functions code, documentation, and behaviour. Let's merge as soon as it passes tests.