diff --git a/NEWS.md b/NEWS.md index b7b1f11..f430c57 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,7 @@ * add new feature: winner take one method for `biproporz()` and `pukelsheim` (#10) * allow non-integer vote counts * round divisors to as few digits as possible +* changed error message for undefined/tied results * fixed bugs and added more real data tests # proporz 1.4.0 diff --git a/R/divisor-check.R b/R/divisor-check.R index 809d74e..7f0b6d2 100644 --- a/R/divisor-check.R +++ b/R/divisor-check.R @@ -9,8 +9,11 @@ check_edge_quotient = function(mtrx_quotient, n_seats, return_indices = FALSE) { if(is.nan(quotient_first_without)) quotient_first_without <- 0 if(quotient_last_with == quotient_first_without) { - indices = which(mtrx_quotient == quotient_last_with, arr.ind = TRUE)[,"col"] - parties = paste0(indices, collapse = " & ") + indices = unique(which(mtrx_quotient == quotient_last_with, arr.ind = TRUE)[,"col"]) + parties = collapse_names(indices) + if(!is.null(colnames(mtrx_quotient))) { + parties <- collapse_names(colnames(mtrx_quotient)[indices]) + } stop("Result is undefined, equal quotient for parties: ", parties, call. = FALSE) } } diff --git a/R/divisor.R b/R/divisor.R index 186c6ab..a3ed898 100644 --- a/R/divisor.R +++ b/R/divisor.R @@ -40,6 +40,7 @@ highest_averages_method = function(votes, n_seats, divisors) { # method mtrx_votes = matrix(rep(votes, each=n_seats), ncol = n_parties) + colnames(mtrx_votes) <- names(votes) mtrx_divisors = matrix(rep(divisors, ncol(mtrx_votes)), ncol = n_parties) mtrx_quotient = mtrx_votes/mtrx_divisors diff --git a/R/quota.R b/R/quota.R index bd4f54e..38a4f12 100644 --- a/R/quota.R +++ b/R/quota.R @@ -73,8 +73,11 @@ check_equal_entries = function(remainders, ordered_remainders, n_seats_remaining remainder_first_without = remainders[ordered_remainders[n_seats_remaining+1]] if(remainder_last_with == remainder_first_without) { - indices = which(remainders == remainder_last_with, arr.ind = TRUE) - parties = paste0(indices, collapse = " & ") + indices = unique(which(remainders == remainder_last_with, arr.ind = TRUE)) + parties = collapse_names(indices) + if(!is.null(names(remainders))) { + parties <- collapse_names(names(remainders)[indices]) + } stop("Result is undefined, equal remainder for parties: ", parties, call. = FALSE) } diff --git a/R/utils.R b/R/utils.R index 064b0aa..1d486fe 100644 --- a/R/utils.R +++ b/R/utils.R @@ -104,7 +104,11 @@ assert = function(check) { } collapse_names = function(x) { - y = paste(x, collapse = "', '") - y <- paste0("'", y, "'") + if(is.character(x)) { + y = paste(x, collapse = "', '") + y <- paste0("'", y, "'") + } else { + y = paste(x, collapse = ", ") + } return(y) } diff --git a/tests/testthat/test-biproportional.R b/tests/testthat/test-biproportional.R index b4889e1..482dce1 100644 --- a/tests/testthat/test-biproportional.R +++ b/tests/testthat/test-biproportional.R @@ -142,7 +142,10 @@ test_that("undefined result biproportional", { proporz(rowSums(vm), sum(seats), "round")) expect_error_fixed(upper_apportionment(vm, seats), - "Result is undefined, equal quotient for parties: 4 & 6") + "Result is undefined, equal quotient for parties: 4, 6") + + expect_error_fixed(biproporz(uri2020$votes_matrix, uri2020$seats_vector, quorum_any(any_district = 0.7)), + "Result is undefined, equal quotient for parties: 'CVP', 'SPGB', 'FDP', 'SVP'") # manual fix (actual implementation depends on rules) vm4 <- vm6 <- vm diff --git a/tests/testthat/test-proporz.R b/tests/testthat/test-proporz.R index d229edd..88e5b9c 100644 --- a/tests/testthat/test-proporz.R +++ b/tests/testthat/test-proporz.R @@ -86,11 +86,16 @@ test_that("all method names", { test_that("undefined result errors", { expect_error(proporz(c(1, 10, 10), 1, "round"), - "Result is undefined, equal quotient for parties: 2 & 3", fixed = TRUE) + "Result is undefined, equal quotient for parties: 2, 3", fixed = TRUE) + expect_error(proporz(c(1, X = 10, ABC = 10), 1, "round"), + "Result is undefined, equal quotient for parties: 'X', 'ABC'", fixed = TRUE) expect_equal(proporz(c(1, 10, 10), 2, "round"), c(0,1,1)) expect_error(largest_remainder_method(c(10, 10, 0), 1), - "Result is undefined, equal remainder for parties: 1 & 2", + "Result is undefined, equal remainder for parties: 1, 2", + fixed = TRUE) + expect_error(largest_remainder_method(c(A = 10, X = 10, P = 0), 1), + "Result is undefined, equal remainder for parties: 'A', 'X'", fixed = TRUE) }) diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index b77f4db..0685f33 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -53,3 +53,10 @@ test_that("print", { M = biproporz(matrix(c(51,60,63,98,100,102,45,120,144), nrow = 3), 4:6) expect_equal(capture.output(print(M)), capture.output(print(as.matrix(M)))) }) + +test_that("collapse_names", { + x = c("Abc", "XYZ") + y = c(2, 6, 67) + expect_equal(collapse_names(x), "'Abc', 'XYZ'") + expect_equal(collapse_names(y), "2, 6, 67") +})