Skip to content

Error in the documentation for csv.reader #113046

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.

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

Closed
pscabot opened this issue Dec 13, 2023 · 14 comments
Closed

Error in the documentation for csv.reader #113046

pscabot opened this issue Dec 13, 2023 · 14 comments
Labels
docs Documentation in the Doc dir

Comments

@pscabot
Copy link

pscabot commented Dec 13, 2023

Documentation

In the description for csv.reader function it says:

Return a reader object which will iterate over lines in the given csvfile. csvfile can be any object which supports the iterator protocol and returns a string each time its __next__() method is called

(The bold emphasis is mine)

However it returns a list as it's properly documented at csvreader.__next()__

Linked PRs

@pscabot pscabot added the docs Documentation in the Doc dir label Dec 13, 2023
@ericvsmith
Copy link
Member

The bold text refers to the first argument to csv.reader, which is named csvfile. The documentation you refer to is for the return value of csv.reader.

@willingc
Copy link
Contributor

@pscabot Thanks for submitting the docs issue. As @ericvsmith mentions, the existing documentation accurately reflects the implementation.

I recommend taking a look at https://peps.python.org/pep-0305/ as called out in the "see also" box. This should give some additional clarity.

I'm going to go ahead and close this issue. Please feel free to ask for additional clarification of the csv.reader operation if needed.

@willingc willingc closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2023
@terryjreedy terryjreedy reopened this Dec 13, 2023
@terryjreedy
Copy link
Member

I am reopening because the second sentence is a bit confusing and I think it should be replaced, as suggested below. In full, it says

csvfile can be any object which supports the iterator protocol and returns a string each time its __next__() method is called — file objects and list objects are both suitable.

The sentence says that csvfile 'can' ('should', 'must'?) be an iterator of strings, with a __next__ method, but a list csvfile is only an iterable and does not have a __next__ method. This makes it possible to think that 'its' refers back to the returned iterator, which always does have a __next__ method.

I think the replacement should be properly say what 'csvfile' must be, something like:

The csvfile must be an iterable of strings, each with the needed format. File and list objects are the most commonly used.

@ericvsmith
Copy link
Member

I agree the documentation could be improved. @pscabot : Does Terry's change above make sense to you?

@pscabot
Copy link
Author

pscabot commented Dec 14, 2023

@ericvsmith
I believe the confusing part (at least what confused me) is the juxtaposition of the return value and the requirements for csvfile.
I would use @terryjreedy change but also add a new line and a reference:

Return a reader object which will iterate over lines in the given csvfile.  See csvreader.__next__().

The csvfile must be an iterable of strings, each with the needed format. File and list objects are the most commonly used.

@ericvsmith
Copy link
Member

I don't think we should link to csvreader.__next__(), but rather to https://docs.python.org/3/library/csv.html#reader-objects. And I think the link can just be from "reader object" in your first sentence, and delete the second sentence. So (cleaned up a little):

Return a reader object which will iterate over lines in the given csvfile. csvfile must be an iterable of strings, each with the needed format. File and list objects are the most commonly used.

@pscabot
Copy link
Author

pscabot commented Dec 14, 2023

I would still recommend a new line. All other changes are great.

@willingc
Copy link
Contributor

Thanks @terryjreedy for the additions. ❤️

@jeremy-dolan
Copy link
Contributor

jeremy-dolan commented Dec 14, 2023

I don't think the sentence is actually ambiguous as "its" couldn't grammatically refer to the "reader object" in the previous sentence. Regardless, maybe this is less wordy and clearer:

CHANGE: "csvfile can be any object which supports the iterator protocol and returns a string each time its __next__() method is called"...
TO: "csvfile can be any iterable that returns a string each time its __next__() method is called"...

@ericvsmith
Copy link
Member

I think it's confusing to mention __next__, since no one ever directly calls it. The point is that it's an iterable returning csv-formatted strings.

A slight tweak:

Return a reader object which will process lines from the given csvfile. csvfile must be an iterable of strings, each with the needed format. csvfile is most commonly a file-like object or list.

Although "with the needed format" seems a little wishy-washy. Maybe "in csv format"?

@terryjreedy
Copy link
Member

I agree that the link is sufficient, and that part of my proposal was a wishy-washy place-holder. How about "each in the reader's defined csv format" or "each in the csv format defined by the other arguments"? My point here is that there is no one concrete 'csv format', but a family of csv formats.

@ericvsmith
Copy link
Member

I like "each in the reader's defined csv format".

@terryjreedy
Copy link
Member

Making a PR now. Thanks for the discussion here. I far prefer hashing out short doc patches on the issue, without bot noise, than on the PR itself.

terryjreedy added a commit to terryjreedy/cpython that referenced this issue Dec 16, 2023
Clarify nature of csvfile.
@terryjreedy
Copy link
Member

@jeremy-dolan As I said in #113046 (comment), An iterables does not have a __next__ method unless it is also an iterator, and a list of strings, one of two suggested types for csvfile, is not an iterator.

terryjreedy added a commit that referenced this issue Dec 16, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 16, 2023
Clarify nature of csvfile.
(cherry picked from commit 84df317)

Co-authored-by: Terry Jan Reedy <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Dec 16, 2023
Clarify nature of csvfile.
(cherry picked from commit 84df317)

Co-authored-by: Terry Jan Reedy <[email protected]>
terryjreedy added a commit that referenced this issue Dec 16, 2023
Clarify nature of csvfile.
(cherry picked from commit 84df317)

Co-authored-by: Terry Jan Reedy <[email protected]>
terryjreedy added a commit that referenced this issue Dec 16, 2023
Clarify nature of csvfile.
(cherry picked from commit 84df317)

Co-authored-by: Terry Jan Reedy <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
Status: Done
Development

No branches or pull requests

6 participants
@ericvsmith @willingc @terryjreedy @pscabot @jeremy-dolan and others