Skip to content

Commit a0ca9e7

Browse files
gh-132813: Improve error messages for incorrect types and values of csv.Dialect attributes
Make them similar to PyArg_Parse error messages, mention None as a possible value, show a wrong type and the string length.
1 parent feac343 commit a0ca9e7

File tree

3 files changed

+69
-54
lines changed

3 files changed

+69
-54
lines changed

Lib/test/test_csv.py

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,19 +1121,22 @@ class mydialect(csv.Dialect):
11211121
with self.assertRaises(csv.Error) as cm:
11221122
mydialect()
11231123
self.assertEqual(str(cm.exception),
1124-
'"quotechar" must be a 1-character string')
1124+
'"quotechar" must be a unicode character or None, '
1125+
'not a string of length 0')
11251126

11261127
mydialect.quotechar = "''"
11271128
with self.assertRaises(csv.Error) as cm:
11281129
mydialect()
11291130
self.assertEqual(str(cm.exception),
1130-
'"quotechar" must be a 1-character string')
1131+
'"quotechar" must be a unicode character or None, '
1132+
'not a string of length 2')
11311133

11321134
mydialect.quotechar = 4
11331135
with self.assertRaises(csv.Error) as cm:
11341136
mydialect()
11351137
self.assertEqual(str(cm.exception),
1136-
'"quotechar" must be string or None, not int')
1138+
'"quotechar" must be a unicode character or None, '
1139+
'not int')
11371140

11381141
def test_delimiter(self):
11391142
class mydialect(csv.Dialect):
@@ -1150,31 +1153,32 @@ class mydialect(csv.Dialect):
11501153
with self.assertRaises(csv.Error) as cm:
11511154
mydialect()
11521155
self.assertEqual(str(cm.exception),
1153-
'"delimiter" must be a 1-character string')
1156+
'"delimiter" must be a unicode character, '
1157+
'not a string of length 3')
11541158

11551159
mydialect.delimiter = ""
11561160
with self.assertRaises(csv.Error) as cm:
11571161
mydialect()
11581162
self.assertEqual(str(cm.exception),
1159-
'"delimiter" must be a 1-character string')
1163+
'"delimiter" must be a unicode character, not a string of length 0')
11601164

11611165
mydialect.delimiter = b","
11621166
with self.assertRaises(csv.Error) as cm:
11631167
mydialect()
11641168
self.assertEqual(str(cm.exception),
1165-
'"delimiter" must be string, not bytes')
1169+
'"delimiter" must be a unicode character, not bytes')
11661170

11671171
mydialect.delimiter = 4
11681172
with self.assertRaises(csv.Error) as cm:
11691173
mydialect()
11701174
self.assertEqual(str(cm.exception),
1171-
'"delimiter" must be string, not int')
1175+
'"delimiter" must be a unicode character, not int')
11721176

11731177
mydialect.delimiter = None
11741178
with self.assertRaises(csv.Error) as cm:
11751179
mydialect()
11761180
self.assertEqual(str(cm.exception),
1177-
'"delimiter" must be string, not NoneType')
1181+
'"delimiter" must be a unicode character, not NoneType')
11781182

11791183
def test_escapechar(self):
11801184
class mydialect(csv.Dialect):
@@ -1188,20 +1192,32 @@ class mydialect(csv.Dialect):
11881192
self.assertEqual(d.escapechar, "\\")
11891193

11901194
mydialect.escapechar = ""
1191-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'):
1195+
with self.assertRaises(csv.Error) as cm:
11921196
mydialect()
1197+
self.assertEqual(str(cm.exception),
1198+
'"escapechar" must be a unicode character or None, '
1199+
'not a string of length 0')
11931200

11941201
mydialect.escapechar = "**"
1195-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'):
1202+
with self.assertRaises(csv.Error) as cm:
11961203
mydialect()
1204+
self.assertEqual(str(cm.exception),
1205+
'"escapechar" must be a unicode character or None, '
1206+
'not a string of length 2')
11971207

11981208
mydialect.escapechar = b"*"
1199-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not bytes'):
1209+
with self.assertRaises(csv.Error) as cm:
12001210
mydialect()
1211+
self.assertEqual(str(cm.exception),
1212+
'"escapechar" must be a unicode character or None, '
1213+
'not bytes')
12011214

12021215
mydialect.escapechar = 4
1203-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not int'):
1216+
with self.assertRaises(csv.Error) as cm:
12041217
mydialect()
1218+
self.assertEqual(str(cm.exception),
1219+
'"escapechar" must be a unicode character or None, '
1220+
'not int')
12051221

12061222
def test_lineterminator(self):
12071223
class mydialect(csv.Dialect):
@@ -1222,7 +1238,13 @@ class mydialect(csv.Dialect):
12221238
with self.assertRaises(csv.Error) as cm:
12231239
mydialect()
12241240
self.assertEqual(str(cm.exception),
1225-
'"lineterminator" must be a string')
1241+
'"lineterminator" must be a string, not int')
1242+
1243+
mydialect.lineterminator = None
1244+
with self.assertRaises(csv.Error) as cm:
1245+
mydialect()
1246+
self.assertEqual(str(cm.exception),
1247+
'"lineterminator" must be a string, not NoneType')
12261248

12271249
def test_invalid_chars(self):
12281250
def create_invalid(field_name, value, **kwargs):
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Improve error messages for incorrect types and values of :class:`csv.Dialect`
2+
attributes.

Modules/_csv.c

Lines changed: 32 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ _set_int(const char *name, int *target, PyObject *src, int dflt)
237237
int value;
238238
if (!PyLong_CheckExact(src)) {
239239
PyErr_Format(PyExc_TypeError,
240-
"\"%s\" must be an integer", name);
240+
"\"%s\" must be an integer, not %T", name, src);
241241
return -1;
242242
}
243243
value = PyLong_AsInt(src);
@@ -255,27 +255,29 @@ _set_char_or_none(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt
255255
if (src == NULL) {
256256
*target = dflt;
257257
}
258-
else {
258+
else if (src == Py_None) {
259259
*target = NOT_SET;
260-
if (src != Py_None) {
261-
if (!PyUnicode_Check(src)) {
262-
PyErr_Format(PyExc_TypeError,
263-
"\"%s\" must be string or None, not %.200s", name,
264-
Py_TYPE(src)->tp_name);
265-
return -1;
266-
}
267-
Py_ssize_t len = PyUnicode_GetLength(src);
268-
if (len < 0) {
269-
return -1;
270-
}
271-
if (len != 1) {
272-
PyErr_Format(PyExc_TypeError,
273-
"\"%s\" must be a 1-character string",
274-
name);
275-
return -1;
276-
}
277-
*target = PyUnicode_READ_CHAR(src, 0);
260+
}
261+
else {
262+
// similar to PyArg_Parse("C?")
263+
if (!PyUnicode_Check(src)) {
264+
PyErr_Format(PyExc_TypeError,
265+
"\"%s\" must be a unicode character or None, not %T",
266+
name, src);
267+
return -1;
268+
}
269+
Py_ssize_t len = PyUnicode_GetLength(src);
270+
if (len < 0) {
271+
return -1;
278272
}
273+
if (len != 1) {
274+
PyErr_Format(PyExc_TypeError,
275+
"\"%s\" must be a unicode character or None, "
276+
"not a string of length %zd",
277+
name, len);
278+
return -1;
279+
}
280+
*target = PyUnicode_READ_CHAR(src, 0);
279281
}
280282
return 0;
281283
}
@@ -287,20 +289,22 @@ _set_char(const char *name, Py_UCS4 *target, PyObject *src, Py_UCS4 dflt)
287289
*target = dflt;
288290
}
289291
else {
292+
// similar to PyArg_Parse("C")
290293
if (!PyUnicode_Check(src)) {
291294
PyErr_Format(PyExc_TypeError,
292-
"\"%s\" must be string, not %.200s", name,
293-
Py_TYPE(src)->tp_name);
294-
return -1;
295+
"\"%s\" must be a unicode character, not %T",
296+
name, src);
297+
return -1;
295298
}
296299
Py_ssize_t len = PyUnicode_GetLength(src);
297300
if (len < 0) {
298301
return -1;
299302
}
300303
if (len != 1) {
301304
PyErr_Format(PyExc_TypeError,
302-
"\"%s\" must be a 1-character string",
303-
name);
305+
"\"%s\" must be a unicode character, "
306+
"not a string of length %zd",
307+
name, len);
304308
return -1;
305309
}
306310
*target = PyUnicode_READ_CHAR(src, 0);
@@ -314,16 +318,12 @@ _set_str(const char *name, PyObject **target, PyObject *src, const char *dflt)
314318
if (src == NULL)
315319
*target = PyUnicode_DecodeASCII(dflt, strlen(dflt), NULL);
316320
else {
317-
if (src == Py_None)
318-
*target = NULL;
319-
else if (!PyUnicode_Check(src)) {
321+
if (!PyUnicode_Check(src)) {
320322
PyErr_Format(PyExc_TypeError,
321-
"\"%s\" must be a string", name);
323+
"\"%s\" must be a string, not %T", name, src);
322324
return -1;
323325
}
324-
else {
325-
Py_XSETREF(*target, Py_NewRef(src));
326-
}
326+
Py_XSETREF(*target, Py_NewRef(src));
327327
}
328328
return 0;
329329
}
@@ -533,22 +533,13 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
533533
/* validate options */
534534
if (dialect_check_quoting(self->quoting))
535535
goto err;
536-
if (self->delimiter == NOT_SET) {
537-
PyErr_SetString(PyExc_TypeError,
538-
"\"delimiter\" must be a 1-character string");
539-
goto err;
540-
}
541536
if (quotechar == Py_None && quoting == NULL)
542537
self->quoting = QUOTE_NONE;
543538
if (self->quoting != QUOTE_NONE && self->quotechar == NOT_SET) {
544539
PyErr_SetString(PyExc_TypeError,
545540
"quotechar must be set if quoting enabled");
546541
goto err;
547542
}
548-
if (self->lineterminator == NULL) {
549-
PyErr_SetString(PyExc_TypeError, "lineterminator must be set");
550-
goto err;
551-
}
552543
if (dialect_check_char("delimiter", self->delimiter, self, true) ||
553544
dialect_check_char("escapechar", self->escapechar, self,
554545
!self->skipinitialspace) ||

0 commit comments

Comments
 (0)