classification
Title: csv.DictReader argument names documented incorrectly
Type: behavior Stage: resolved
Components: Documentation, Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: James.Salt, benjamin.peterson, berker.peksag, docs@python, eric.araujo, gbengeult, larry, maaz92, petere, python-dev, sandro.tosi, serhiy.storchaka
Priority: low Keywords: easy, patch

Created on 2012-09-24 16:53 by petere, last changed 2017-01-07 06:32 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
james.salt-corrections_to_documentation_in_csv-01.txt James.Salt, 2012-09-28 15:26 Diff of change generated using hg diff > |filename| review
csv_csvfile_arg.patch serhiy.storchaka, 2012-09-28 17:06 review
csv.rst-patch gbengeult, 2017-01-04 02:57 Diff of Doc/library/csv.rst review
Messages (14)
msg171161 - (view) Author: Peter Eisentraut (petere) * Date: 2012-09-24 16:53
The documentation for the csv.DictReader constructor is

.. class:: DictReader(csvfile, fieldnames=None, restkey=None, restval=None, dialect='excel', *args, **kwds)

but the implementation is

def __init__(self, f, fieldnames=None, restkey=None, restval=None, dialect="excel", *args, **kwds):

The name of the first argument is documented incorrectly, leading to surprise errors when attempting to use key word arguments.
msg171491 - (view) Author: James Salt (James.Salt) Date: 2012-09-28 15:26
This diff shows a correction of the documentation to reflect the name for the csv file used in the implementation - this change seemed like the easiest and simplest thing to do and avoids potential backwards incompatibility issues.
msg171494 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-09-28 15:31
Eric, iirc you're the de facto csv guy?  Does this seem reasonable to you?
msg171495 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-09-28 15:36
I’m not a dedicated maintainer for csv (for one thing I don’t know C :), I just happen to reply to many bug reports.

This doc patch looks good to me.
msg171507 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-09-28 17:06
It is not consistent with reader and writer.

I propose to close the issue as "won't fix" or change the argument name in the sources. Unfortunately, Python syntax does not allow to specify the positional-only arguments.
msg171510 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-09-28 17:35
I don’t understand the proposal.  Changing the code has a high bar; here it would break code without benefit.  Fixing the doc so that it reflects the code accurately is a no-cost improvement.  (Inconsistency is unfortunate, but we’ll have to live with that.)
msg171516 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-09-28 18:26
Changing the documentation is changing the specification, it has the higher bar than changing the implementation. It can break the alternative implementations.

This is unlikely to break somebody's existing code, because the one who used the argument as a keyword argument should have paid attention to the discrepancy of the documentation. But fixing the name, we will be difficult to change it in the future (to eliminate inconsistency).

I believe this change is too serious for bugfix. This is the case when never is better than *right* now.
msg171521 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-09-28 18:53
I disagree that we can never fix argument names in the docs, as we have done it already.  You raise a good point however, that we should check what other VMs do.  I would not be surprised if they followed the code instead of the docs.
msg281543 - (view) Author: Mohammad Maaz Khan (maaz92) Date: 2016-11-23 08:16
Hi Éric, I think the documentation should be changed to match the arguments' names.
msg281552 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-11-23 10:33
James Salt's patch looks good to me, but it doesn't apply cleanly anymore.

For the record, PyPy uses the same parameter name as Lib/csv.py: https://bitbucket.org/pypy/pypy/src/55a9404c80d6557854cac092addd92a6e0b683cc/lib-python/2.7/csv.py?at=default&fileviewer=file-view-default#csv.py-74
msg284606 - (view) Author: Greg Bengeult (gbengeult) * Date: 2017-01-04 02:57
Here's a revised patch file for Python 3.7. This is my first ever submission to an open source project, so please be gentle.
msg284613 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-01-04 05:55
Welcome and thanks for the patch, Greg. csv.rst-patch looks good to me. I will commit it this week if there are no objections.

Documentation updates can go into all active branches so I'm adding 3.5 and 3.6 back (we can pass 2.7 at this point since this is a trivial change)
msg284892 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2017-01-07 06:31
Thanks for the patches James and Greg!
msg284893 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-07 06:32
New changeset 198edd926751 by Berker Peksag in branch '3.6':
Issue #16026: Fix parameter names of DictReader and DictWriter
https://hg.python.org/cpython/rev/198edd926751

New changeset 63c5531cfdf7 by Berker Peksag in branch 'default':
Issue #16026: Merge from 3.6
https://hg.python.org/cpython/rev/63c5531cfdf7
History
Date User Action Args
2018-02-07 07:00:20serhiy.storchakalinkissue32785 superseder
2017-01-07 06:32:23python-devsetnosy: + python-dev
messages: + msg284893
2017-01-07 06:31:33berker.peksagsetstatus: open -> closed
versions: - Python 3.5
messages: + msg284892

resolution: fixed
stage: patch review -> resolved
2017-01-04 05:55:19berker.peksagsetstage: needs patch -> patch review
messages: + msg284613
versions: + Python 3.5, Python 3.6
2017-01-04 02:57:54gbengeultsetfiles: + csv.rst-patch
versions: - Python 2.7, Python 3.5, Python 3.6
nosy: + gbengeult

messages: + msg284606
2016-11-23 10:33:51berker.peksagsetpriority: normal -> low
versions: + Python 3.5, Python 3.6, Python 3.7, - Python 3.2, Python 3.3
nosy: + berker.peksag

messages: + msg281552

keywords: + easy, - needs review
2016-11-23 08:16:58maaz92setnosy: + maaz92
messages: + msg281543
2012-09-28 18:53:57eric.araujosetkeywords: + patch, needs review, - 3.3regression
2012-09-28 18:53:28eric.araujosetkeywords: + 3.3regression, - patch, easy
nosy: + benjamin.peterson
messages: + msg171521

2012-09-28 18:26:23serhiy.storchakasetmessages: + msg171516
2012-09-28 17:35:34eric.araujosetmessages: + msg171510
2012-09-28 17:06:41serhiy.storchakasetfiles: + csv_csvfile_arg.patch

nosy: + serhiy.storchaka
messages: + msg171507

keywords: + patch
2012-09-28 15:36:11eric.araujosetnosy: + sandro.tosi
messages: + msg171495
2012-09-28 15:31:21larrysetnosy: + eric.araujo, larry
messages: + msg171494
2012-09-28 15:26:09James.Saltsetfiles: + james.salt-corrections_to_documentation_in_csv-01.txt
nosy: + James.Salt
messages: + msg171491

2012-09-25 01:03:33eric.araujosetkeywords: + easy
stage: needs patch
versions: - Python 2.6, Python 3.1, Python 3.4
2012-09-24 16:53:03peterecreate