gh-108202: `calendar`: Document `TextCalendar` methods by StanFromIreland · Pull Request #127608 · python/cpython · GitHub

#108202

Issue: Docs: the calendar module lacks documentation for several APIs #108202

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

Some initial comments. (I'll tag Erlend again once I consider this ready from a triage standpoint.)

It's probably worth converting the phrase "string representation" into a reference to the str class.

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

As per ZeroIntenisities suggestion

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Ok, I'll refrain from nitpicking. I don't know enough about calendar to give this a green check, so I'll just tag Erlend.

@erlend-aasland All done now. In the future do you prefer a PR for every function like hugokvs maybe it is easier to review?

No, do not create one PR for every function. Just don't mix different types of changes in a PR (nor in an issue). This is all explained in clear text in the devguide.

@erlend-aasland ok. Is this PR ok for you then? Are there any other changes you want before you review? This PR was originally focused on just the functions but then expanded with suggestions from other reviewers, I have reverted all of them.

Is this PR ok for you then?

I'm torn about these changes. If you take the time to read through this issue's history, you'll find that it is in fact pretty controversial. Some core devs actively oppose documenting methods that could/should be considered helpers and/or private APIs. Other core devs lean towards that every (intentional or unintentional) public API should be documented. There is no happy path.

I guess I'm fine with this; since other similar calendar formatting helpers have been documented, we might as well document the rest of them.

This PR was originally focused on just the functions but then expanded with suggestions from other reviewers, I have reverted all of them.

I see; I'm sorry about the extra work. I did not read through all of the previous remarks. I prefer to align strictly with our recommended development guidelines and strictly keep any type of change atomic. This implies:

not mixing multiple bugfixes
not mixing bugfixes and features
not mixing multiple features (related or unrelated)
not mixing different types of documentation enhancements or "bugfixes"

As with any kind of rule, there will be exceptions. But exceptions are exceptions; if they become the new norm, our guidelines become moot.

Keeping changes atomic has several positive effects:

changes are easier to review
code changes are easier to debug1
backporting is made easier
reverting problematic changes is easier

note that git bisect becomes pointless if you mix bugfixes and/or features in a single PR ↩

@picnixz: if you are satisfied with this, feel free to land it.

A last markup change and we're good.

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

(FTR, I still don't have the commit rights so you'll still need to hit the merge button @erlend-aasland :'))

(FTR, I still don't have the commit rights [...]

What?! Outrageous! 😄

…ythonGH-127608) Document the following TextCalendar methods: - formatday() - formatmonthname() - formatweekday() - formatweekheader() (cherry picked from commit bb73426) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>

GH-128440 is a backport of this pull request to the 3.13 branch.

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

…H-127608) (#128441) Document the following TextCalendar methods: - formatday() - formatmonthname() - formatweekday() - formatweekheader() (cherry picked from commit bb73426) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>

…H-127608) (#128440) Document the following TextCalendar methods: - formatday() - formatmonthname() - formatweekday() - formatweekheader() (cherry picked from commit bb73426) Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>

…ython#127608) Document the following TextCalendar methods: - formatday() - formatmonthname() - formatweekday() - formatweekheader()

Document formatday and prepare

fb2fee6

bedevere-app Bot added docs Documentation in the Doc dir skip news labels Dec 4, 2024

bedevere-app Bot mentioned this pull request Dec 4, 2024

Docs: the calendar module lacks documentation for several APIs #108202

Open

Update calendar.rst

8c17c78

StanFromIreland marked this pull request as ready for review December 5, 2024 16:27

bedevere-app Bot added the awaiting review label Dec 5, 2024

This comment was marked as outdated.

Sign in to view

ZeroIntensity reviewed Dec 10, 2024

View reviewed changes

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

ZeroIntensity added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Dec 10, 2024

StanFromIreland and others added 5 commits December 10, 2024 16:04

Update Doc/library/calendar.rst

e3a14b0

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Update Doc/library/calendar.rst

a41d652

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Update Doc/library/calendar.rst

238befb

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Update Doc/library/calendar.rst

781876c

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

Change string to :class:str

60b524f

As per ZeroIntenisities suggestion

StanFromIreland requested a review from ZeroIntensity December 10, 2024 16:14

ZeroIntensity reviewed Dec 10, 2024

View reviewed changes

Comment thread Doc/library/calendar.rst Outdated

Antony3293 mentioned this pull request Dec 10, 2024

Gato #127804

Closed

Update Doc/library/calendar.rst

6ed0c74

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

StanFromIreland requested a review from ZeroIntensity December 10, 2024 22:56

ZeroIntensity reviewed Dec 11, 2024

View reviewed changes

ZeroIntensity requested a review from erlend-aasland December 11, 2024 15:47

eendebakpt reviewed Dec 17, 2024

View reviewed changes

Comment thread Doc/library/calendar.rst Outdated

eendebakpt reviewed Dec 17, 2024

View reviewed changes

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

Update calendar.rst

60c5572

StanFromIreland requested review from ZeroIntensity and eendebakpt December 29, 2024 10:33

eendebakpt approved these changes Dec 29, 2024

View reviewed changes

Comment thread Doc/library/calendar.rst Outdated

Comment thread Doc/library/calendar.rst Outdated

bedevere-app Bot removed the awaiting review label Dec 29, 2024

Update calendar.rst

54ceef6

erlend-aasland approved these changes Jan 2, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels Jan 2, 2025

picnixz reviewed Jan 3, 2025

View reviewed changes

Comment thread Doc/library/calendar.rst Outdated

Update Doc/library/calendar.rst

bfbfaf6

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

erlend-aasland assigned picnixz Jan 3, 2025

picnixz approved these changes Jan 3, 2025

View reviewed changes

erlend-aasland merged commit bb73426 into python:main Jan 3, 2025

This comment was marked as outdated.

Sign in to view

bedevere-app Bot removed the awaiting merge label Jan 3, 2025

bedevere-app Bot removed the needs backport to 3.13 only security fixes label Jan 3, 2025

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jan 3, 2025

StanFromIreland deleted the patch-4 branch February 11, 2025 16:42

gh-108202: `calendar`: Document `TextCalendar` methods by StanFromIreland · Pull Request #127608 · python/cpython · GitHub

Repository navigation

gh-108202: calendar: Document TextCalendar methods - #127608

gh-108202: calendar: Document TextCalendar methods#127608erlend-aasland merged 14 commits intopython:mainpython/cpython:mainfrom StanFromIreland:patch-4StanFromIreland/cpython:patch-4Copy head branch name to clipboard

Conversation

StanFromIreland commented Dec 4, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

StanFromIreland commented Jan 1, 2025

Uh oh!

erlend-aasland commented Jan 1, 2025

Uh oh!

StanFromIreland commented Jan 1, 2025

Uh oh!

erlend-aasland commented Jan 2, 2025

Uh oh!

erlend-aasland commented Jan 2, 2025

Footnotes

Uh oh!

erlend-aasland commented Jan 2, 2025

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

picnixz commented Jan 3, 2025

Uh oh!

erlend-aasland commented Jan 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

bedevere-app Bot commented Jan 3, 2025

Uh oh!

bedevere-app Bot commented Jan 3, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gh-108202: calendar: Document TextCalendar methods#127608
erlend-aasland merged 14 commits into
python:mainfrom
StanFromIreland:patch-4

StanFromIreland commented Dec 4, 2024 •
edited by github-actions Bot
Loading

erlend-aasland commented Jan 3, 2025 •
edited
Loading