Fix broken doc links in Calendar.new docstring - #1732
Open
Mohammed-Ahmed7 wants to merge 1 commit into
Open
Conversation
Mohammed-Ahmed7
requested review from
SashankBhamidi,
angatha,
niccokunzmann and
stevepiercy
as code owners
August 27, 2026 21:48
github-actions
Bot
added
the
ai-suspicion
Contributor
|
This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments. |
Contributor
|
Profile summary: Full profile |
Mohammed-Ahmed7
force-pushed
the
fix/calendar-doc-links
branch
from
August 27, 2026 22:00
f535e65 to
97a025c
Compare
Availability was missing its full module path. DEFAULT_PRODID pointed to an undocumented constant; replaced with plain code formatting instead. AI: Claude Sonnet 5 (via Claude Code), used as a tutor to learn Sphinx link syntax and verify the build. I wrote, edited, and verified all changes myself.
Mohammed-Ahmed7
force-pushed
the
fix/calendar-doc-links
branch
from
August 27, 2026 22:04
97a025c to
051ecd4
Compare
stevepiercy
requested changes
Aug 28, 2026
stevepiercy
left a comment
stevepiercy
left a comment
Member
There was a problem hiding this comment.
@Mohammed-Ahmed7 thanks for your contribution. It made me think it would be shame not to link to DEFAULT_PRODID, so see what I suggested. Also there's a shorthand syntax, and the news should be updated to align with the suggestions. Would you please take care?
|
|
||
|
|
||
| DEFAULT_PRODID = f"-//collective//icalendar//{__version__}//EN" | ||
|
|
Member
There was a problem hiding this comment.
Actually, let's document this value. Then the link works.
Suggested change
| """The value for :attr:`~icalendar.cal.calendar.Calendar.prodid` when it's not provided.""" | |
| generates a `prodid` in the format of "-//organization//name//language". | ||
| If ``None`` and ``organization`` is not provided, sets it to | ||
| :attr:`~icalendar.cal.calendar.DEFAULT_PRODID`. | ||
| ``DEFAULT_PRODID``. |
Member
There was a problem hiding this comment.
Please revert this change, and see my suggestion above.
https://github.com/collective/icalendar/pull/1732/changes#r3879155959
| @property | ||
| def availabilities(self) -> list[Availability]: | ||
| """All :class:`Availability` components in the calendar. | ||
| """All :class:`Availability <icalendar.cal.availability.Availability>` components in the calendar. |
Member
There was a problem hiding this comment.
See the "object only" syntax example at https://icalendar.readthedocs.io/en/latest/contribute/documentation/style-guide.html#cross-reference-python-objects.
Suggested change
| """All :class:`Availability <icalendar.cal.availability.Availability>` components in the calendar. | |
| """All :class:`~icalendar.cal.availability.Availability` components in the calendar. |
Comment on lines
+1
to
+10
| Fixed two broken documentation cross-reference links in the | ||
| :meth:`~icalendar.cal.calendar.Calendar.new` docstring: the | ||
| :class:`~icalendar.cal.availability.Availability` class reference | ||
| was missing its full module path, and the ``DEFAULT_PRODID`` | ||
| reference pointed to an undocumented module-level constant and | ||
| was changed to plain code formatting instead. | ||
|
|
||
| I used AI to assist me with this change. | ||
|
|
||
| @Mohammed-Ahmed7 |
Member
There was a problem hiding this comment.
Please put this on a single line, and update it to reflect the suggestions below.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue
Description
Fixed two broken documentation links in the Calendar.new docstring
(calendar.py). The
Availabilityclass reference was missing itsfull module path, so Sphinx couldn't resolve it. The
DEFAULT_PRODIDreference pointed to an undocumented module-level constant with no
linkable target, so it was changed to plain code formatting instead —
confirmed against the style guide and issue #1072, neither of which
documents a way to link module-level constants.
Verified locally with a Sphinx nitpicky build: warnings dropped from
70 to 68, both references now render correctly. Also ran the full
test suite; two unrelated pre-existing failures were found (external
link rot in linkcheck, and a local timezone-database quirk on my
machine) — neither relates to this change.
No test added: this fix corrects a Sphinx cross-reference syntax
error in a docstring, which the pytest suite doesn't cover — it's
verified via the Sphinx nitpicky build instead (see above).
Checklist