gh-101100: Fix references in csv docs #114658

smontanaro · 2024-01-27T18:15:40Z

This is my first attempt at this. Hopefully I've done it correctly. I would appreciate checks on my changes and the mechanics (is gh-101100 the correct issue to reference?). Thanks to @hugovk for helping me get started. There are two types of changes herein:

The usual reference fixes.
A small wording change to the Dialect documentation reflecting that it really doesn't have any callable methods, and – in particular – lacks a validate method. (Validation is implicit when instantiating the class.)

With these changes, nitpicky mode in Sphinx seems happy with csv.rst.

📚 Documentation preview 📚: https://cpython-previews--114658.org.readthedocs.build/

hugovk

Thank you!

is #101100 the correct issue to reference?

Yep :)

With these changes, nitpicky mode in Sphinx seems happy with csv.rst.

And the CI has failed... But in a good way! It says:

Congratulations! You improved:

Doc/library/csv.rst

Please remove from Doc/tools/.nitignore

Doing this means the CI will also prevent new warnings from appearing in this file.

Doc/library/csv.rst

hugovk · 2024-01-27T20:14:56Z

Doc/library/csv.rst

@@ -492,9 +492,9 @@ DictReader objects have the following public attribute:
 Writer Objects
 --------------

-:class:`Writer` objects (:class:`DictWriter` instances and objects returned by
+:class:`Writer<writer>` objects (:class:`DictWriter` instances and objects returned by


We usually add a space here:

Suggested change

:class:`Writer<writer>` objects (:class:`DictWriter` instances and objects returned by

:class:`Writer <writer>` objects (:class:`DictWriter` instances and objects returned by

Although, I don't see a Writer class with capital W, is it only capitalised because it begins the sentence? If so, I'd either reword so it's not first, or leave it lowercase.

Yes, I left it capitalized. I suspect the original author may have done it that way. I just added a proper reference. The csv module dates from the days when types (written in C) and classes (written in Python) were not quite the same thing. Type names generally weren't capitalized (I think, hence csv.writer), while classes were, hence csv.DictWriter.

Co-authored-by: Hugo van Kemenade <[email protected]>

smontanaro · 2024-01-27T20:42:56Z

@hugovk I think I made all the suggested changes...

hugovk

Looks good, thanks!

Just want to double check the write.

hugovk · 2024-01-28T12:51:42Z

Doc/library/csv.rst

@@ -88,7 +88,7 @@ The :mod:`csv` module defines the following functions:

   Return a writer object responsible for converting the user's data into delimited
   strings on the given file-like object.  *csvfile* can be any object with a
-   :func:`write` method.  If *csvfile* is a file object, it should be opened with
+   :meth:`~io.TextIOBase.write` method.  If *csvfile* is a file object, it should be opened with


@serhiy-storchaka Is this the correct one for write in this case?

I guess it is.

Good question. I mulled that over a bit before using TextIOBase, eventually deciding that since CSV files are supposed to be opened as plain text files since Python 3, that was the most reasonable link destination.

You can also explicitly mention that write() should accept a string argument (not bytes). And the same for the reader.

serhiy-storchaka

LGTM.

Perhaps we can rename csvwriter to writer, but it is a separate issue, and it may have pitfalls.

serhiy-storchaka · 2024-01-28T13:49:37Z

Doc/library/csv.rst

@@ -88,7 +88,7 @@ The :mod:`csv` module defines the following functions:

   Return a writer object responsible for converting the user's data into delimited
   strings on the given file-like object.  *csvfile* can be any object with a
-   :func:`write` method.  If *csvfile* is a file object, it should be opened with
+   :meth:`~io.TextIOBase.write` method.  If *csvfile* is a file object, it should be opened with


I guess it is.

smontanaro · 2024-01-28T14:32:55Z

Perhaps we can rename csvwriter to writer, but it is a separate issue, and it may have pitfalls.

Since there is no type called csvwriter, what pitfalls would correcting those references be?

serhiy-storchaka · 2024-01-28T14:51:15Z

csv.writer is not a class, csv.writer() returns an instance of _csv.Writer class which is not exposed itself. writerow() is defined as a method of a fictional class csvwriter, but actually it is a description of writerow() in unrelated classes _csv.Writer and csv.DictWriter. csv.DictWriter is not a subclass of _csv.Writer or csv.writer (which is not even a class). All so complicated, that I cannot predict what Sphinx will produce. We have to try and see at the result. All can just work, or produce glitches from which we could not know how to get rid.

hugovk · 2024-01-28T16:53:26Z

In the spirit of incremental changes, shall we merge this as-is (it does what it set out to do: fix reference warnings), and make a new PR for any further changes?

miss-islington-app · 2024-01-30T21:54:40Z

Thanks @smontanaro for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington-app · 2024-01-30T21:54:43Z

Sorry, @smontanaro and @serhiy-storchaka, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 3911b42cc0d404e0eac87fce30b740b08618ff06 3.12

miss-islington-app · 2024-01-30T21:54:57Z

Sorry, @smontanaro and @serhiy-storchaka, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 3911b42cc0d404e0eac87fce30b740b08618ff06 3.11

Co-authored-by: Hugo van Kemenade <[email protected]> (cherry picked from commit 3911b42)

smontanaro · 2024-01-30T22:53:14Z

@serhiy-storchaka @hugovk I attempted to create the two pull requests. Please take a look and let me know if I missed something or resolved the .nitignore conflict incorrectly (quite possible).

zware · 2024-01-30T23:09:08Z

I haven't reviewed the content of GH-114771 (3.12) or GH-114773 (3.11), but the diffs look a lot better with the right base branches :)

smontanaro · 2024-01-30T23:36:50Z

Not sure what you're referring to. I was just executing the cherry-picker commands the bedevere not have. Did I mess something up?

zware · 2024-01-30T23:42:29Z

I don't know if it was you or cherry_picker, but the PRs were based on main rather than their respective 3.12 or 3.11 branch.

The PR branches themselves were fine, though. That is, the error was in the creation of the PRs, not the creation of the backports.

smontanaro · 2024-01-30T23:44:02Z

That's weird. I did run the commands from my 3.12 and 3.11 repos.

bedevere-app · 2024-01-31T09:23:11Z

GH-114771 is a backport of this pull request to the 3.12 branch.

bedevere-app · 2024-01-31T09:23:23Z

GH-114773 is a backport of this pull request to the 3.11 branch.

Co-authored-by: Hugo van Kemenade <[email protected]> (cherry picked from commit 3911b42)

Co-authored-by: Hugo van Kemenade <[email protected]>

some ref fixes for csv doc

d7814ef

bedevere-app bot added awaiting review docs Documentation in the Doc dir skip news labels Jan 27, 2024

bedevere-app bot mentioned this pull request Jan 27, 2024

Fix all Sphinx reference warnings in the documentation #101100

Open

hugovk reviewed Jan 27, 2024

View reviewed changes

hugovk added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Jan 27, 2024

smontanaro and others added 3 commits January 27, 2024 14:30

got the seal of approval I guess

601b095

Apply suggestions from code review

8faab33

Co-authored-by: Hugo van Kemenade <[email protected]>

per hugovk's suggestion

f3bf42a

hugovk approved these changes Jan 28, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Jan 28, 2024

serhiy-storchaka approved these changes Jan 28, 2024

View reviewed changes

Merge branch 'main' into doc-fix

3c3ed52

serhiy-storchaka enabled auto-merge (squash) January 30, 2024 21:51

serhiy-storchaka merged commit 3911b42 into python:main Jan 30, 2024

bedevere-app bot removed the awaiting merge label Jan 30, 2024

miss-islington-app bot assigned serhiy-storchaka Jan 30, 2024

smontanaro added a commit to smontanaro/cpython that referenced this pull request Jan 30, 2024

pythongh-101100: Fix references in csv docs (pythonGH-114658)

78be88c

Co-authored-by: Hugo van Kemenade <[email protected]> (cherry picked from commit 3911b42)

serhiy-storchaka changed the title ~~gh-101100: ref fixes for csv doc~~ gh-101100: Fix references in csv docs Jan 31, 2024

bedevere-app bot removed the needs backport to 3.12 only security fixes label Jan 31, 2024

bedevere-app bot removed the needs backport to 3.11 only security fixes label Jan 31, 2024

serhiy-storchaka pushed a commit that referenced this pull request Jan 31, 2024

[3.11] gh-101100: Fix references in csv docs (GH-114658) (GH-114773)

a38bc7c

Co-authored-by: Hugo van Kemenade <[email protected]> (cherry picked from commit 3911b42)

serhiy-storchaka pushed a commit that referenced this pull request Jan 31, 2024

[3.12] gh-101100: Fix references in csv docs (GH-114658) (GH-114771)

bb578f6

Co-authored-by: Hugo van Kemenade <[email protected]> (cherry picked from commit 3911b42)

aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024

pythongh-101100: Fix references in csv docs (pythonGH-114658)

0a7b75d

Co-authored-by: Hugo van Kemenade <[email protected]>

	:class:`Writer<writer>` objects (:class:`DictWriter` instances and objects returned by
	:class:`Writer <writer>` objects (:class:`DictWriter` instances and objects returned by

Uh oh!

gh-101100: Fix references in csv docs #114658

gh-101100: Fix references in csv docs #114658

Uh oh!

Conversation

smontanaro commented Jan 27, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hugovk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugovk Jan 27, 2024

Choose a reason for hiding this comment

Uh oh!

smontanaro Jan 27, 2024

Choose a reason for hiding this comment

Uh oh!

smontanaro commented Jan 27, 2024

Uh oh!

hugovk left a comment

Choose a reason for hiding this comment

Uh oh!

hugovk Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

smontanaro Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jan 28, 2024

Choose a reason for hiding this comment

Uh oh!

smontanaro commented Jan 28, 2024

Uh oh!

serhiy-storchaka commented Jan 28, 2024

Uh oh!

hugovk commented Jan 28, 2024

Uh oh!

miss-islington-app bot commented Jan 30, 2024

Uh oh!

miss-islington-app bot commented Jan 30, 2024

Uh oh!

miss-islington-app bot commented Jan 30, 2024

Uh oh!

smontanaro commented Jan 30, 2024

Uh oh!

zware commented Jan 30, 2024

Uh oh!

smontanaro commented Jan 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zware commented Jan 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smontanaro commented Jan 30, 2024

Uh oh!

bedevere-app bot commented Jan 31, 2024

Uh oh!

bedevere-app bot commented Jan 31, 2024

Uh oh!

Uh oh!

smontanaro commented Jan 27, 2024 •

edited by github-actions bot

Loading

hugovk left a comment •

edited

Loading

smontanaro commented Jan 30, 2024 •

edited

Loading

zware commented Jan 30, 2024 •

edited

Loading