Skip to content

bpo-34197: Make _csv.Dialect attributes booleans. #8440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
serhiy-storchaka merged 1 commit into from
Jul 26, 2018
Merged

bpo-34197: Make _csv.Dialect attributes booleans. #8440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 19 additions & 21 deletions Lib/test/test_csv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ def _test_default_attrs(self, ctor, *args):
obj = ctor(*args)
# Check defaults
self.assertEqual(obj.dialect.delimiter, ',')
self.assertEqual(obj.dialect.doublequote, True)
self.assertIs(obj.dialect.doublequote, True)
self.assertEqual(obj.dialect.escapechar, None)
self.assertEqual(obj.dialect.lineterminator, "\r\n")
self.assertEqual(obj.dialect.quotechar, '"')
self.assertEqual(obj.dialect.quoting, csv.QUOTE_MINIMAL)
self.assertEqual(obj.dialect.skipinitialspace, False)
self.assertEqual(obj.dialect.strict, False)
self.assertIs(obj.dialect.skipinitialspace, False)
self.assertIs(obj.dialect.strict, False)
# Try deleting or changing attributes (they are read-only)
self.assertRaises(AttributeError, delattr, obj.dialect, 'delimiter')
self.assertRaises(AttributeError, setattr, obj.dialect, 'delimiter', ':')
Expand All @@ -76,13 +76,13 @@ def _test_kw_attrs(self, ctor, *args):
strict=True)
obj = ctor(*args, **kwargs)
self.assertEqual(obj.dialect.delimiter, ':')
self.assertEqual(obj.dialect.doublequote, False)
self.assertIs(obj.dialect.doublequote, False)
self.assertEqual(obj.dialect.escapechar, '\\')
self.assertEqual(obj.dialect.lineterminator, "\r")
self.assertEqual(obj.dialect.quotechar, '*')
self.assertEqual(obj.dialect.quoting, csv.QUOTE_NONE)
self.assertEqual(obj.dialect.skipinitialspace, True)
self.assertEqual(obj.dialect.strict, True)
self.assertIs(obj.dialect.skipinitialspace, True)
self.assertIs(obj.dialect.strict, True)

def test_reader_kw_attrs(self):
self._test_kw_attrs(csv.reader, [])
Expand All @@ -104,13 +104,13 @@ class dialect:
args = args + (dialect,)
obj = ctor(*args)
self.assertEqual(obj.dialect.delimiter, '-')
self.assertEqual(obj.dialect.doublequote, False)
self.assertIs(obj.dialect.doublequote, False)
self.assertEqual(obj.dialect.escapechar, '^')
self.assertEqual(obj.dialect.lineterminator, "$")
self.assertEqual(obj.dialect.quotechar, '#')
self.assertEqual(obj.dialect.quoting, csv.QUOTE_ALL)
self.assertEqual(obj.dialect.skipinitialspace, True)
self.assertEqual(obj.dialect.strict, False)
self.assertIs(obj.dialect.skipinitialspace, True)
self.assertIs(obj.dialect.strict, False)

def test_reader_dialect_attrs(self):
self._test_dialect_attrs(csv.reader, [])
Expand Down Expand Up @@ -976,15 +976,13 @@ class TestSniffer(unittest.TestCase):

def test_has_header(self):
sniffer = csv.Sniffer()
self.assertEqual(sniffer.has_header(self.sample1), False)
self.assertEqual(sniffer.has_header(self.header1 + self.sample1),
True)
self.assertIs(sniffer.has_header(self.sample1), False)
self.assertIs(sniffer.has_header(self.header1 + self.sample1), True)

def test_has_header_regex_special_delimiter(self):
sniffer = csv.Sniffer()
self.assertEqual(sniffer.has_header(self.sample8), False)
self.assertEqual(sniffer.has_header(self.header2 + self.sample8),
True)
self.assertIs(sniffer.has_header(self.sample8), False)
self.assertIs(sniffer.has_header(self.header2 + self.sample8), True)

def test_guess_quote_and_delimiter(self):
sniffer = csv.Sniffer()
Expand All @@ -1001,12 +999,12 @@ def test_sniff(self):
dialect = sniffer.sniff(self.sample1)
self.assertEqual(dialect.delimiter, ",")
self.assertEqual(dialect.quotechar, '"')
self.assertEqual(dialect.skipinitialspace, True)
self.assertIs(dialect.skipinitialspace, True)

dialect = sniffer.sniff(self.sample2)
self.assertEqual(dialect.delimiter, ":")
self.assertEqual(dialect.quotechar, "'")
self.assertEqual(dialect.skipinitialspace, False)
self.assertIs(dialect.skipinitialspace, False)

def test_delimiters(self):
sniffer = csv.Sniffer()
Expand Down Expand Up @@ -1068,7 +1066,7 @@ def test_create_read(self):
delta = rc-lastrc
lastrc = rc
# if csv.reader() leaks, last delta should be 3 or more
self.assertEqual(delta < 3, True)
self.assertLess(delta, 3)

def test_create_write(self):
delta = 0
Expand All @@ -1084,7 +1082,7 @@ def test_create_write(self):
delta = rc-lastrc
lastrc = rc
# if csv.writer() leaks, last delta should be 3 or more
self.assertEqual(delta < 3, True)
self.assertLess(delta, 3)

def test_read(self):
delta = 0
Expand All @@ -1100,7 +1098,7 @@ def test_read(self):
delta = rc-lastrc
lastrc = rc
# if reader leaks during read, delta should be 5 or more
self.assertEqual(delta < 5, True)
self.assertLess(delta, 5)

def test_write(self):
delta = 0
Expand All @@ -1117,7 +1115,7 @@ def test_write(self):
delta = rc-lastrc
lastrc = rc
# if writer leaks during write, last delta should be 5 or more
self.assertEqual(delta < 5, True)
self.assertLess(delta, 5)

class TestUnicode(unittest.TestCase):

Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2018-07-23-14-12-28.bpo-34197.7yFSP5.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Attributes *skipinitialspace*, *doublequote* and *strict* of the *dialect*
attribute of the :mod:`csv` reader are now :class:`bool` instances instead
of integers 0 or 1.
31 changes: 16 additions & 15 deletions Modules/_csv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module instead.

#include "Python.h"
#include "structmember.h"
#include <stdbool.h>

@methane methane Jul 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdbool.h.html

How about using bool instead of char? Isn't it portable?

@serhiy-storchaka serhiy-storchaka Jul 25, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T_BOOL requires char. I have doubts that _Bool has always the same size and alignment as char.

@methane methane Jul 25, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense.



typedef struct {
Expand Down Expand Up @@ -74,15 +75,15 @@ static const StyleDesc quote_styles[] = {
typedef struct {
PyObject_HEAD

int doublequote; /* is " represented by ""? */
Py_UCS4 delimiter; /* field separator */
Py_UCS4 quotechar; /* quote character */
Py_UCS4 escapechar; /* escape character */
int skipinitialspace; /* ignore spaces following delimiter? */
PyObject *lineterminator; /* string to write between records */
char doublequote; /* is " represented by ""? */
char skipinitialspace; /* ignore spaces following delimiter? */
char strict; /* raise exception on bad CSV */
int quoting; /* style of quoting to write */
Py_UCS4 delimiter; /* field separator */
Py_UCS4 quotechar; /* quote character */
Py_UCS4 escapechar; /* escape character */
PyObject *lineterminator; /* string to write between records */

int strict; /* raise exception on bad CSV */
} DialectObj;

static PyTypeObject Dialect_Type;
Expand Down Expand Up @@ -189,15 +190,15 @@ Dialect_get_quoting(DialectObj *self)
}

static int
_set_bool(const char *name, int *target, PyObject *src, int dflt)
_set_bool(const char *name, char *target, PyObject *src, bool dflt)
{
if (src == NULL)
*target = dflt;
else {
int b = PyObject_IsTrue(src);
if (b < 0)
return -1;
*target = b;
*target = (char)b;
}
return 0;
}
Expand Down Expand Up @@ -292,9 +293,9 @@ dialect_check_quoting(int quoting)
#define D_OFF(x) offsetof(DialectObj, x)

static struct PyMemberDef Dialect_memberlist[] = {
{ "skipinitialspace", T_INT, D_OFF(skipinitialspace), READONLY },
{ "doublequote", T_INT, D_OFF(doublequote), READONLY },
{ "strict", T_INT, D_OFF(strict), READONLY },
{ "skipinitialspace", T_BOOL, D_OFF(skipinitialspace), READONLY },
{ "doublequote", T_BOOL, D_OFF(doublequote), READONLY },
{ "strict", T_BOOL, D_OFF(strict), READONLY },
{ NULL }
};

Expand Down Expand Up @@ -411,13 +412,13 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
if (meth(name, target, src, dflt)) \
goto err
DIASET(_set_char, "delimiter", &self->delimiter, delimiter, ',');
DIASET(_set_bool, "doublequote", &self->doublequote, doublequote, 1);
DIASET(_set_bool, "doublequote", &self->doublequote, doublequote, true);
DIASET(_set_char, "escapechar", &self->escapechar, escapechar, 0);
DIASET(_set_str, "lineterminator", &self->lineterminator, lineterminator, "\r\n");
DIASET(_set_char, "quotechar", &self->quotechar, quotechar, '"');
DIASET(_set_int, "quoting", &self->quoting, quoting, QUOTE_MINIMAL);
DIASET(_set_bool, "skipinitialspace", &self->skipinitialspace, skipinitialspace, 0);
DIASET(_set_bool, "strict", &self->strict, strict, 0);
DIASET(_set_bool, "skipinitialspace", &self->skipinitialspace, skipinitialspace, false);
DIASET(_set_bool, "strict", &self->strict, strict, false);

/* validate options */
if (dialect_check_quoting(self->quoting))
Expand Down