Skip to content

Commit ca28acf

Browse files
thomasp85thomasp85
committed
Unknown colours are errors
1 parent 6915865 commit ca28acf

File tree

3 files changed

+26
-25
lines changed

3 files changed

+26
-25
lines changed

‎src/encode.cpp‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ inline int cap0255(int x) {
2626
}
2727
inline int hex2int(const int x) {
2828
if (!std::isxdigit(x)) {
29-
Rf_error("Invalid hexadecimal digit");
29+
Rf_errorcall(R_NilValue, "Invalid hexadecimal digit");
3030
}
3131
return (x & 0xf) + (x >> 6) + ((x >> 6) << 3);
3232
}
@@ -43,7 +43,7 @@ template <typename From>
4343
SEXP encode_impl(SEXP colour, SEXP alpha, SEXP white) {
4444
int n_channels = dimension<From>();
4545
if (Rf_ncols(colour) < n_channels) {
46-
Rf_error("Colour in this format must contain at least %i columns", n_channels);
46+
Rf_errorcall(R_NilValue, "Colour in this format must contain at least %i columns", n_channels);
4747
}
4848
static ColorSpace::Rgb rgb;
4949
ColorSpace::XyzConverter::SetWhiteReference(REAL(white)[0], REAL(white)[1], REAL(white)[2]);
@@ -151,7 +151,7 @@ SEXP encode_impl(SEXP colour, SEXP alpha, SEXP white) {
151151
template<>
152152
SEXP encode_impl<ColorSpace::Rgb>(SEXP colour, SEXP alpha, SEXP white) {
153153
if (Rf_ncols(colour) < 3) {
154-
Rf_error("Colour in RGB format must contain at least 3 columns");
154+
Rf_errorcall(R_NilValue, "Colour in RGB format must contain at least 3 columns");
155155
}
156156
int n = Rf_nrows(colour);
157157
SEXP codes = PROTECT(Rf_allocVector(STRSXP, n));
@@ -316,7 +316,7 @@ SEXP load_colour_names_c(SEXP name, SEXP value) {
316316
ColourMap& named_colours = get_named_colours();
317317
int n = Rf_length(name);
318318
if (n != Rf_ncols(value)) {
319-
Rf_error("name and value must have the same length");
319+
Rf_errorcall(R_NilValue, "name and value must have the same length");
320320
}
321321
int* values = INTEGER(value);
322322
int it = 0;
@@ -373,7 +373,7 @@ SEXP decode_impl(SEXP codes, SEXP alpha, SEXP white, SEXP na) {
373373
nchar = strlen(col);
374374
has_alpha = nchar == 9;
375375
if (!has_alpha && nchar != 7) {
376-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
376+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
377377
}
378378
rgb.r = hex2int(col[1]) * 16 + hex2int(col[2]);
379379
rgb.g = hex2int(col[3]) * 16 + hex2int(col[4]);
@@ -387,7 +387,7 @@ SEXP decode_impl(SEXP codes, SEXP alpha, SEXP white, SEXP na) {
387387
} else {
388388
ColourMap::iterator it = named_colours.find(prepare_code(col));
389389
if (it == named_colours.end()) {
390-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
390+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
391391
colours_p[offset1 + i] = R_NaReal;
392392
colours_p[offset2 + i] = R_NaReal;
393393
colours_p[offset3 + i] = R_NaReal;
@@ -462,7 +462,7 @@ SEXP decode_impl<ColorSpace::Rgb>(SEXP codes, SEXP alpha, SEXP white, SEXP na) {
462462
nchar = strlen(col);
463463
has_alpha = nchar == 9;
464464
if (!has_alpha && nchar != 7) {
465-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
465+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
466466
}
467467
r = hex2int(col[1]) * 16 + hex2int(col[2]);
468468
g = hex2int(col[3]) * 16 + hex2int(col[4]);
@@ -476,7 +476,7 @@ SEXP decode_impl<ColorSpace::Rgb>(SEXP codes, SEXP alpha, SEXP white, SEXP na) {
476476
} else {
477477
ColourMap::iterator it = named_colours.find(prepare_code(col));
478478
if (it == named_colours.end()) {
479-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
479+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
480480
if (get_alpha) {
481481
colours_d[offset1 + i] = R_NaReal;
482482
colours_d[offset2 + i] = R_NaReal;
@@ -572,7 +572,7 @@ SEXP encode_channel_impl(SEXP codes, SEXP channel, SEXP value, SEXP op, SEXP whi
572572
if (col[0] == '#') {
573573
nchar = strlen(col);
574574
if (nchar != 9 && nchar != 7) {
575-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
575+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
576576
}
577577
rgb.r = hex2int(col[1]) * 16 + hex2int(col[2]);
578578
rgb.g = hex2int(col[3]) * 16 + hex2int(col[4]);
@@ -581,7 +581,7 @@ SEXP encode_channel_impl(SEXP codes, SEXP channel, SEXP value, SEXP op, SEXP whi
581581
} else {
582582
ColourMap::iterator it = named_colours.find(prepare_code(col));
583583
if (it == named_colours.end()) {
584-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
584+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
585585
SET_STRING_ELT(ret, i, R_NaString);
586586
continue;
587587
}
@@ -659,13 +659,13 @@ SEXP encode_channel_impl<ColorSpace::Rgb>(SEXP codes, SEXP channel, SEXP value,
659659
if (col[0] == '#') {
660660
nchar = strlen(col);
661661
if (nchar != 9 && nchar != 7) {
662-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
662+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
663663
}
664664
strcpy(buffera, col);
665665
} else {
666666
ColourMap::iterator it = named_colours.find(prepare_code(col));
667667
if (it == named_colours.end()) {
668-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
668+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
669669
SET_STRING_ELT(ret, i, R_NaString);
670670
continue;
671671
}
@@ -744,7 +744,7 @@ SEXP encode_alpha_impl(SEXP codes, SEXP value, SEXP op) {
744744
if (col[0] == '#') {
745745
nchar = strlen(col);
746746
if (nchar != 9 && nchar != 7) {
747-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
747+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
748748
}
749749
strcpy(buffera, col);
750750
if (strlen(buffera) == 7) {
@@ -755,7 +755,7 @@ SEXP encode_alpha_impl(SEXP codes, SEXP value, SEXP op) {
755755
} else {
756756
ColourMap::iterator it = named_colours.find(prepare_code(col));
757757
if (it == named_colours.end()) {
758-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
758+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
759759
SET_STRING_ELT(ret, i, R_NaString);
760760
continue;
761761
}
@@ -841,15 +841,15 @@ SEXP decode_channel_impl(SEXP codes, SEXP channel, SEXP white) {
841841
if (col[0] == '#') {
842842
nchar = strlen(col);
843843
if (nchar != 9 && nchar != 7) {
844-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
844+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
845845
}
846846
rgb.r = hex2int(col[1]) * 16 + hex2int(col[2]);
847847
rgb.g = hex2int(col[3]) * 16 + hex2int(col[4]);
848848
rgb.b = hex2int(col[5]) * 16 + hex2int(col[6]);
849849
} else {
850850
ColourMap::iterator it = named_colours.find(prepare_code(col));
851851
if (it == named_colours.end()) {
852-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
852+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
853853
ret_p[i] = R_NaReal;
854854
continue;
855855
}
@@ -889,7 +889,7 @@ SEXP decode_channel_impl<ColorSpace::Rgb>(SEXP codes, SEXP channel, SEXP white)
889889
if (col[0] == '#') {
890890
nchar = strlen(col);
891891
if (nchar != 9 && nchar != 7) {
892-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
892+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
893893
}
894894
switch (chan) {
895895
case 1:
@@ -905,7 +905,7 @@ SEXP decode_channel_impl<ColorSpace::Rgb>(SEXP codes, SEXP channel, SEXP white)
905905
} else {
906906
ColourMap::iterator it = named_colours.find(prepare_code(col));
907907
if (it == named_colours.end()) {
908-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
908+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
909909
ret_p[i] = R_NaInt;
910910
continue;
911911
}
@@ -951,7 +951,7 @@ SEXP decode_alpha_impl(SEXP codes) {
951951
nchar = strlen(col);
952952
has_alpha = nchar == 9;
953953
if (!has_alpha && nchar != 7) {
954-
Rf_error("Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
954+
Rf_errorcall(R_NilValue, "Malformed colour string `%s`. Must contain either 6 or 8 hex values", col);
955955
}
956956
if (has_alpha) {
957957
val = (hex2int(col[7]) * 16 + hex2int(col[8])) / 255.0;
@@ -961,7 +961,7 @@ SEXP decode_alpha_impl(SEXP codes) {
961961
} else {
962962
ColourMap::iterator it = named_colours.find(prepare_code(col));
963963
if (it == named_colours.end()) {
964-
Rf_warningcall(R_NilValue, "Unknown colour name: %s", col);
964+
Rf_errorcall(R_NilValue, "Unknown colour name: %s", col);
965965
ret_p[i] = R_NaReal;
966966
continue;
967967
}

‎src/farver.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ SEXP convert_dispatch_impl(SEXP colour, SEXP white_from, SEXP white_to) {
1414
int ncol = dimension<Space_From>();
1515
// check that the dimensions of the input match the colour space
1616
if (ncol > Rf_ncols(colour)) {
17-
Rf_error("colourspace requires %d values", ncol);
17+
Rf_errorcall(R_NilValue, "colourspace requires %d values", ncol);
1818
}
1919
int ncol_out = dimension<Space_To>();
2020

@@ -142,10 +142,10 @@ SEXP compare_dispatch_impl(SEXP from, SEXP to, int dist, bool sym, SEXP white_fr
142142

143143
// check that the dimensions of the input match the colour space
144144
if (from_col > Rf_ncols(from)) {
145-
Rf_error("colourspace requires %d values", from_col);
145+
Rf_errorcall(R_NilValue, "colourspace requires %d values", from_col);
146146
}
147147
if (to_col > Rf_ncols(to)) {
148-
Rf_error("colourspace requires %d values", to_col);
148+
Rf_errorcall(R_NilValue, "colourspace requires %d values", to_col);
149149
}
150150
double wf1 = REAL(white_from)[0];
151151
double wf2 = REAL(white_from)[1];

‎tests/testthat/test-non-finite.R‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@ bad_colours <- cbind(
77

88
test_that("non-finite values are treated correctly", {
99
expect_error(decode_colour(bad_codes), 'Malformed colour string `#345`')
10-
suppressWarnings(expect_true(all(is.na(decode_colour(bad_codes[-1]))[3:4, ])))
11-
suppressWarnings(expect_false(any(is.na(decode_colour(bad_codes[-1]))[1:2, ])))
10+
expect_error(decode_colour(bad_codes[-1]), 'Unknown colour name: test')
11+
expect_true(all(is.na(decode_colour(bad_codes[-c(1, 4)]))[3, ]))
12+
expect_false(any(is.na(decode_colour(bad_codes[-c(1, 4)]))[1:2, ]))
1213

1314
expect_true(all(is.na(encode_colour(bad_colours))[2:5]))
1415
expect_false(any(is.na(encode_colour(bad_colours))[c(1,6)]))

0 commit comments

Comments
 (0)