classification
Title: csv module: Add return value to DictWriter.writeheader
Type: enhancement Stage: resolved
Components: Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: skip.montanaro Nosy List: SilentGhost, ashishnitinpatil, berker.peksag, lsowen, mblahay, r.david.murray, rhettinger, skip.montanaro, vstinner, zach.ware
Priority: normal Keywords: easy, patch

Created on 2016-07-12 15:14 by lsowen, last changed 2019-05-10 01:52 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
issue_27497_add_return_to_DictWriter_writeheader.patch ashishnitinpatil, 2016-09-04 17:07 Patch for issue 27497 review
issue_27497_add_return_to_DictWriter_writeheader_with_documentation_changes.patch ashishnitinpatil, 2016-09-04 17:09 Patch for issue 27497 with suggested documentation changes review
issue_27497_add_return_to_DictWriter_writeheader_v2.patch ashishnitinpatil, 2016-09-05 15:23 Patch for issue 27497 with suggested documentation changes and unittests review
Pull Requests
URL Status Linked Edit
PR 12306 merged remi.lapeyre, 2019-03-13 10:01
Messages (22)
msg270248 - (view) Author: Logan (lsowen) Date: 2016-07-12 15:14
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)
msg270250 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2016-07-12 15:31
This seems like a reasonable request, but could only be done in 3.6 as it would be a new feature.
msg270268 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-07-12 22:04
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?
msg270276 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-07-12 23:50
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.
msg270340 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2016-07-13 20:15
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?
msg270341 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-07-13 20:19
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.
msg270346 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-07-13 21:32
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?
msg270363 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-07-14 04:15
By the way, there is an open ticket about changing the recipe in Django documentation: https://code.djangoproject.com/ticket/26040
msg270388 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2016-07-14 10:01
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.
msg270389 - (view) Author: Logan (lsowen) Date: 2016-07-14 10:06
@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.
msg270407 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-07-14 12:38
I'm now ok to add the return, but only in Python 3.6. It would be a minor new "feature".
msg274370 - (view) Author: Ashish Nitin Patil (ashishnitinpatil) * Date: 2016-09-04 17:07
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.
msg274371 - (view) Author: Ashish Nitin Patil (ashishnitinpatil) * Date: 2016-09-04 17:09
Also, I noticed in the documentation for `csv.DictWriter` (https://docs.python.org/3/library/csv.html#csv.DictWriter.writeheader) does not contain the following line -
     to the writer's file object, formatted according to the current dialect.
Although it is there for the other 2 methods (`writerow` & `writerows`) and since `writeheader` uses `writerow` in turn, I believe the said line should be included for the `writeheader` documentation as well. I have created another patch with both the changes. I ran the Doc build (make html) and have checked the addition of the said lines looks good & does not break anything.

I can make another thread if required, if you feel both the changes can't be accommodated in the same issue.
msg274398 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-09-05 12:31
Thanks! The patch lacks a test case (you can add it into Lib/test/test_csv.py) and a versionchanged notice in documentation (see http://cpython-devguide.readthedocs.io/en/latest/documenting.html#paragraph-level-markup for details.)
msg274400 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2016-09-05 14:16
Also, a change in behavior has to be very carefully considered, even
though it passes all existing test cases (and presumably the one(s)
you'll add). The change may well only be acceptable for the next minor
release of 3.x.

On Mon, Sep 5, 2016 at 7:31 AM, Berker Peksag <report@bugs.python.org> wrote:
>
> Berker Peksag added the comment:
>
> Thanks! The patch lacks a test case (you can add it into Lib/test/test_csv.py) and a versionchanged notice in documentation (see http://cpython-devguide.readthedocs.io/en/latest/documenting.html#paragraph-level-markup for details.)
>
> ----------
> stage: needs patch -> patch review
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue27497>
> _______________________________________
msg274402 - (view) Author: Ashish Nitin Patil (ashishnitinpatil) * Date: 2016-09-05 15:23
Thank you Berker & Skip for your review & additional inputs.
I have added a (probably not so good) unittest for testing the newly added *return*.
I have also added the corresponding *versionchanged* info to the documentation as suggested by Berker.

Will keep these in mind while making a patch next time :)
Kindly let me know if there are any further changes required.
msg341556 - (view) Author: Michael Blahay (mblahay) * Date: 2019-05-06 16:29
I would like to drive this to conclusion since it appears this issue has not had any real activity in a while. First thing that needs to be determined is whether this enhancement is still desirable. Who can answer this?
msg341559 - (view) Author: SilentGhost (SilentGhost) * (Python triager) Date: 2019-05-06 16:33
Michael, there is a up-to-date PR that's waiting for approval.
msg341566 - (view) Author: Michael Blahay (mblahay) * Date: 2019-05-06 17:08
Ah ha, so there is. I'm glad this one will get closed out. Sorry for noob mistake.
msg341793 - (view) Author: Skip Montanaro (skip.montanaro) * (Python triager) Date: 2019-05-07 18:11
I think this is ready to go. I added a comment to PR12306. As I am no longer a committer, I'm not sure what the next step is.
msg342034 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-10 01:50
New changeset fce5ff1e18b522cf52379934a6560583d840e7f9 by Victor Stinner (Rémi Lapeyre) in branch 'master':
bpo-27497: Add return value to csv.DictWriter.writeheader (GH-12306)
https://github.com/python/cpython/commit/fce5ff1e18b522cf52379934a6560583d840e7f9
msg342035 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-10 01:52
I merged PR 12306. Thanks Ashish Nitin Patil for the initial patch, thanks Rémi Lapeyre to converting it to a PR, thanks Logan for the initial bug report ;-)

Sadly, this change is a new feature and so cannot be backported to 2.7 or 3.7. The workaround is override the method:

    def writeheader(self):
        header = dict(zip(self.fieldnames, self.fieldnames))
        return self.writerow(header)
History
Date User Action Args
2019-05-10 01:52:50vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg342035

stage: patch review -> resolved
2019-05-10 01:50:14vstinnersetmessages: + msg342034
2019-05-07 18:11:52skip.montanarosetmessages: + msg341793
2019-05-06 17:08:47mblahaysetmessages: + msg341566
2019-05-06 16:33:21SilentGhostsetnosy: + SilentGhost
messages: + msg341559
2019-05-06 16:29:19mblahaysetnosy: + mblahay
messages: + msg341556
2019-03-13 10:02:45remi.lapeyresetversions: + Python 3.8, - Python 3.6
2019-03-13 10:01:35remi.lapeyresetpull_requests: + pull_request12282
2016-09-05 15:23:42ashishnitinpatilsetfiles: + issue_27497_add_return_to_DictWriter_writeheader_v2.patch

messages: + msg274402
2016-09-05 14:16:22skip.montanarosetmessages: + msg274400
2016-09-05 12:31:25berker.peksagsetmessages: + msg274398
stage: needs patch -> patch review
2016-09-04 17:09:43ashishnitinpatilsetfiles: + issue_27497_add_return_to_DictWriter_writeheader_with_documentation_changes.patch

messages: + msg274371
2016-09-04 17:07:51ashishnitinpatilsetfiles: + issue_27497_add_return_to_DictWriter_writeheader.patch

nosy: + ashishnitinpatil
messages: + msg274370

keywords: + patch
2016-07-14 12:38:44vstinnersetmessages: + msg270407
2016-07-14 10:06:32lsowensetmessages: + msg270389
2016-07-14 10:01:31skip.montanarosetmessages: + msg270388
2016-07-14 04:15:14berker.peksagsetnosy: + berker.peksag
messages: + msg270363
2016-07-13 21:32:21vstinnersetmessages: + msg270346
2016-07-13 20:19:34r.david.murraysetmessages: + msg270341
2016-07-13 20:15:06rhettingersetassignee: skip.montanaro

messages: + msg270340
nosy: + skip.montanaro, rhettinger
2016-07-12 23:50:16r.david.murraysetnosy: + r.david.murray
messages: + msg270276
2016-07-12 22:04:55vstinnersetnosy: + vstinner
messages: + msg270268
2016-07-12 15:31:17zach.waresetversions: - Python 3.5
nosy: + zach.ware

messages: + msg270250

keywords: + easy
stage: needs patch
2016-07-12 15:14:08lsowencreate