csv module: Add return value to DictWriter.writeheader

[lsowen]

BPO	27497
Nosy	@smontanaro, @rhettinger, @vstinner, @bitdancer, @berkerpeksag, @zware, @ashishnitinpatil, @lsowen, @mblahay
PRs	bpo-27497: Add return value to csv.DictWriter.writeheader #12306
Files	issue_27497_add_return_to_DictWriter_writeheader.patch: Patch for issue 27497 issue_27497_add_return_to_DictWriter_writeheader_with_documentation_changes.patch: Patch for issue 27497 with suggested documentation changes issue_27497_add_return_to_DictWriter_writeheader_v2.patch: Patch for issue 27497 with suggested documentation changes and unittests

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/smontanaro'
closed_at = <Date 2019-05-10.01:52:50.157>
created_at = <Date 2016-07-12.15:14:08.016>
labels = ['easy', 'type-feature', '3.8']
title = 'csv module: Add return value to DictWriter.writeheader'
updated_at = <Date 2019-05-10.01:52:50.156>
user = 'https://github.com/lsowen'

bugs.python.org fields:

activity = <Date 2019-05-10.01:52:50.156>
actor = 'vstinner'
assignee = 'skip.montanaro'
closed = True
closed_date = <Date 2019-05-10.01:52:50.157>
closer = 'vstinner'
components = []
creation = <Date 2016-07-12.15:14:08.016>
creator = 'lsowen'
dependencies = []
files = ['44364', '44365', '44372']
hgrepos = []
issue_num = 27497
keywords = ['patch', 'easy']
message_count = 22.0
messages = ['270248', '270250', '270268', '270276', '270340', '270341', '270346', '270363', '270388', '270389', '270407', '274370', '274371', '274398', '274400', '274402', '341556', '341559', '341566', '341793', '342034', '342035']
nosy_count = 10.0
nosy_names = ['skip.montanaro', 'rhettinger', 'vstinner', 'r.david.murray', 'SilentGhost', 'berker.peksag', 'zach.ware', 'ashishnitinpatil', 'lsowen', 'mblahay']
pr_nums = ['12306']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27497'
versions = ['Python 3.8']

[lsowen]

Currently, DictWriter.writeheader() is defined like:

    def writeheader(self):
        header = dict(zip(self.fieldnames, self.fieldnames))
        self.writerow(header)

It would be useful to have it return the value of writerow():

    def writeheader(self):
        header = dict(zip(self.fieldnames, self.fieldnames))
        return self.writerow(header)

This would useful because:

1. It would match the behavior of DictWriter.writerow
2. It would enable DictWriter.writeheader to be used in within a generator function (ala https://docs.djangoproject.com/en/1.9/howto/outputting-csv/#streaming-large-csv-files)

[zware]

This seems like a reasonable request, but could only be done in 3.6 as it would be a new feature.

[vstinner]

It doesn't seem right to me.

The writer should be blocking: write *all* data, don't use partial write.

Supporting partial write (non-blocking files) requires more changes, and it isn't worth it.

Here the problem is that the function doesn't support partial write. Each time, it tries to write the full content.

What is your use case?

[bitdancer]

It isn't documented that writer.writeline returns anything, but what it actually returns is whatever the write method of the underlying file object returns. Obviously (given the linked example), this fact is being used. There is value in consistency, so I think we should make this change.

[rhettinger]

It is a bit irritating to have this small API inconsistency, but I'm a little wary of propagating this undocumented and untested behavior especially given Victor's concern about whether it makes any sense in this context.

Skip, what do you think?

[bitdancer]

It doesn't make sense when the return value is that provided by io.write. It does make sense in the context of the linked example (a psuedo-file object). However, it *is* an undocumented API, even if people are using it.

[vstinner]

Oh. I missed the django recipe to implement "streaming csv". It uses an
Echo object, its write() method returns its parameter.

This recipe looks like an abuse of the API, but I understand that there is
a need for a kind of "partial write": give control to I/O to the caller.

Maybe we should extend the API?

[berkerpeksag]

By the way, there is an open ticket about changing the recipe in Django documentation: https://code.djangoproject.com/ticket/26040

[smontanaro]

I agree writeheader() should have returned the value of writerow(), but have no opinion on the more esoteric stuff you're discussing. I think you could argue that adding the "return" would be a bug fix.

Personally, I long ago got in the habit of using the writerow(dict(zip(fh, fh))) idiom, didn't catch on at the time that writeheader() was even available in 2.7, and still just use the idiom most of the time. It's a one-liner. The OP could easily use that in contexts where writeheader() doesn't do the right thing.

[lsowen]

@berker.peksag: Good catch, thank you. In my code base I'm using a slightly different implementation which does use an iterator, so I didn't even catch that the default django example was incorrect.

@skip.montanaro: That is exactly the one-liner I'm currently using. Thank you for documenting it here in case anyone else is looking for the same.

[vstinner]

I'm now ok to add the return, but only in Python 3.6. It would be a minor new "feature".

[ashishnitinpatil]

Hi, I have added the "return" to the writeheader method. I ran the tests for the csv library (./python -m test -v test_csv) and got no errors (Ran 101 tests in 0.322s; 1 test OK.).

Kindly review the patch.