homepage

The sections on DictReader and DictWriter do not give information on what the type is of the parameter 'fieldnames'. The source code comments that it is a 'list of keys for the dict'.

Fieldnames is referred to as a sequence, which does define its type. That said, defining the type earlier in that paragraph would probably be a good thing.

I am new to contributing to python, and I would like to take a shot a creating a patch for this.

I have created a patch for this documentation issue. Could you please review this for me and tell me what you think?

I did not fully realize the meaning of it being a sequence as it is referred to a little later indeed. Which implies both a tuple and a list can be used, right? So then the patch might look like:
+ The *fieldnames* parameter is a list or tuple of keys used for the dict.

It looks like you are right. I have updated the patch to reflect that it could be a list or a tuple.

Well, there is a reason why the term 'sequence' was used. It covers lists, tuples...and anything else that implements the Sequence ABC (http://docs.python.org/library/collections.html#collections-abstract-base-classes).
The word 'sequence' could be made a link to that section by doing:
 :mod:`sequence <collections.abc>`
in the Python3 docs (it would need to be a :ref: to the subsection in the python2 docs, I think).

Here is a patch for DictReader that adds a mod link to the sequence abstract as requested. Please review this if you could and let me know what you think. Note: This patch is for python 3 and if you like how I have done it on here I will go ahead and create a patch that covers python 2 as well.

I'd like to combine the two approaches: mention that fieldnames is a sequence (with the link) as soon as fieldnames is introduced in the paragraph, analogous to what you did in the first patch. Whether you then make what becomes the second occurrence of the word sequence also a link is a matter of taste, but I probably wouldn't.

Ok, I have take the approach I used with the original patch and applied it to this one, listing that fieldnames is a sequence. Then, I added a link to the collections abstract on that instance of the word sequence. I did this for both DictReader and DictWriter as I did in the original patch. Would this be what you think is needed?

This looks pretty good. In the DictReader part, it might be good to add something analogous to the DictWriter clarifying what the order of the sequence means. Something like "a :mod:`sequence <collections.abc>` whose elements are associated with the fields of the input data in order, and which become the keys of the resulting dictionary". If think you have a way to improve that wording, please try, as I'm not entirely happy with it :)

What about if I put "The *fieldnames* parameter is a :mod:`sequence <collections.abc>` whose elements are associated with the fields of the input data in order. These elements become the keys of the resulting dictionary." It contains all of the information that you have but it feels more to the point when you separate it as two sentences. The attached patch reflect these changes.

Yes, that does sound better. If you can make an equivalent 2.7 patch I will apply them.

Here is the equivalent 2.7 patch. I used a relative link with the format `sequence <collections.html>`_ in order to link to the collections page. I tried to use the reference syntax but I could find the subsection that I needed to reference in the documentation. If you would like for me to go that route I don't mind trying to figure it out, this is just the way I was able to get the documentation compiling and working correctly.

That is supposed to say I couldn't find the subsection that I needed to reference in the documentation.

I just realized that my two last updates on this ticket could be sort of confusing. So here is what I did for the 2.7 patch. First I copied the wording that I used for the 3.3/4 patch and then I created a relative link using the syntax `sequence <collections.html>`_ because I couldn't find a way to get the :ref: syntax to work correctly. I have applied this patch to the 2.7 branch and compiled the html documentation. When I tested it the link does work correctly. It goes to the collections page of the documentation. If you have time could you review the patch for 2.7 for me? (The patch is named DictReader_DictWriter_python2_NewWording.patch.)

I figured out what you meant, but thanks for the clarification.
The section of interest in 2.7 is near the bottom of the page you linked to. It already has a Sphinx section reference (collections-abstract-base-classes), so you can use :ref: to link to it.

Great! Thank you for the reference name. I have changed the patch to use a ref link. Here is the new patch.

One more nitpick: is it the sequence [of keys] that identif_ies_ the order, or is it the keys that identif_y_ the order? Not being a native English speaker, I'd opt for the first choice.
Thank you both for your meticulous attention for details!

Is there anything else that should be added to this patch? I don't mean to bug you guys just want to make sure that everything is right with it so that if and or when it is applied it will apply without any problems. Also, if there is anything else I should change I am definitely open to changing anything that is needed.

New changeset a5d4660c6cb6 by R David Murray in branch '3.3':
#20628: make it clear that DictReader/Writer *fieldnames* is a Sequence.
http://hg.python.org/cpython/rev/a5d4660c6cb6
New changeset 9f036047187b by R David Murray in branch '3.3':
#20628: wrap lines to < 80.
http://hg.python.org/cpython/rev/9f036047187b
New changeset 5eb7e29df762 by R David Murray in branch 'default':
Merge #20628: make it clear that DictReader/Writer *fieldnames* is a Sequence.
http://hg.python.org/cpython/rev/5eb7e29df762
New changeset 0e77dd295a88 by R David Murray in branch '2.7':
#20628: make it clear that DictReader/Writer *fieldnames* is a Sequence.
http://hg.python.org/cpython/rev/0e77dd295a88
New changeset 0926adcc335c by R David Murray in branch '2.7':
#20628: wrap lines to < 80.
http://hg.python.org/cpython/rev/0926adcc335c 

Applied. Thanks, Sean.