Skip to content

Commit df98a47

Browse files
serhiy-storchakaserhiy-storchaka
authored
pythongh-132813: Improve error messages for incorrect types and values of csv.Dialog attributes (pythonGH-133241)
Make them similar to PyArg_Parse error messages, mention None as a possible value, show a wrong type and the string length.
1 parent e814f43 commit df98a47

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
@@ -1122,19 +1122,22 @@ class mydialect(csv.Dialect):
11221122
with self.assertRaises(csv.Error) as cm:
11231123
mydialect()
11241124
self.assertEqual(str(cm.exception),
1125-
'"quotechar" must be a 1-character string')
1125+
'"quotechar" must be a unicode character or None, '
1126+
'not a string of length 0')
11261127

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

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

11391142
def test_delimiter(self):
11401143
class mydialect(csv.Dialect):
@@ -1151,31 +1154,32 @@ class mydialect(csv.Dialect):
11511154
with self.assertRaises(csv.Error) as cm:
11521155
mydialect()
11531156
self.assertEqual(str(cm.exception),
1154-
'"delimiter" must be a 1-character string')
1157+
'"delimiter" must be a unicode character, '
1158+
'not a string of length 3')
11551159

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

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

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

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

11801184
def test_escapechar(self):
11811185
class mydialect(csv.Dialect):
@@ -1189,20 +1193,32 @@ class mydialect(csv.Dialect):
11891193
self.assertEqual(d.escapechar, "\\")
11901194

11911195
mydialect.escapechar = ""
1192-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'):
1196+
with self.assertRaises(csv.Error) as cm:
11931197
mydialect()
1198+
self.assertEqual(str(cm.exception),
1199+
'"escapechar" must be a unicode character or None, '
1200+
'not a string of length 0')
11941201

11951202
mydialect.escapechar = "**"
1196-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be a 1-character string'):
1203+
with self.assertRaises(csv.Error) as cm:
11971204
mydialect()
1205+
self.assertEqual(str(cm.exception),
1206+
'"escapechar" must be a unicode character or None, '
1207+
'not a string of length 2')
11981208

11991209
mydialect.escapechar = b"*"
1200-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not bytes'):
1210+
with self.assertRaises(csv.Error) as cm:
12011211
mydialect()
1212+
self.assertEqual(str(cm.exception),
1213+
'"escapechar" must be a unicode character or None, '
1214+
'not bytes')
12021215

12031216
mydialect.escapechar = 4
1204-
with self.assertRaisesRegex(csv.Error, '"escapechar" must be string or None, not int'):
1217+
with self.assertRaises(csv.Error) as cm:
12051218
mydialect()
1219+
self.assertEqual(str(cm.exception),
1220+
'"escapechar" must be a unicode character or None, '
1221+
'not int')
12061222

12071223
def test_lineterminator(self):
12081224
class mydialect(csv.Dialect):
@@ -1223,7 +1239,13 @@ class mydialect(csv.Dialect):
12231239
with self.assertRaises(csv.Error) as cm:
12241240
mydialect()
12251241
self.assertEqual(str(cm.exception),
1226-
'"lineterminator" must be a string')
1242+
'"lineterminator" must be a string, not int')
1243+
1244+
mydialect.lineterminator = None
1245+
with self.assertRaises(csv.Error) as cm:
1246+
mydialect()
1247+
self.assertEqual(str(cm.exception),
1248+
'"lineterminator" must be a string, not NoneType')
12271249

12281250
def test_invalid_chars(self):
12291251
def create_invalid(field_name, value, **kwargs):

‎Misc/NEWS.d/next/Library/2025-05-01-10-56-44.gh-issue-132813.rKurvp.rst‎

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)