Skip to content

docs: replace RFC copy-paste docstrings with Pythonic docstrings#1385

Open
ykd007 wants to merge 4 commits into
collective:mainfrom
ykd007:fix/issue-1244-pythonic-docstrings
Open

docs: replace RFC copy-paste docstrings with Pythonic docstrings#1385
ykd007 wants to merge 4 commits into
collective:mainfrom
ykd007:fix/issue-1244-pythonic-docstrings

Conversation

@ykd007
Copy link
Copy Markdown

@ykd007 ykd007 commented May 14, 2026

See #1244

Rewrites RFC copy-paste docstrings across Alarm, Component, and Calendar into clean Pythonic format with concise summaries, examples, and see-also references.

Updated (12 properties across 3 files):

  • Alarm: ACKNOWLEDGED, TRIGGER
  • Component: DTSTAMP/stamp, LAST_MODIFIED, CREATED
  • Calendar: calendar_name, description, color, prodid, version, calscale, method, refresh_interval

All 3 files pass py_compile syntax check.

@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community Bot commented May 14, 2026

@stevepiercy
Copy link
Copy Markdown
Member

@ykd007 would you please resolve the merge conflicts by moving your change log entry into the new format that we adopted in 7.1.2, as documented at https://icalendar.readthedocs.io/en/stable/contribute/index.html#change-log-entry-format?

@stevepiercy
Copy link
Copy Markdown
Member

@ykd007 would you also edit your PR description from Closes #1244 to See #1244? We don't want to close it until all the docstrings are checked off the list.

Copy link
Copy Markdown
Member

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is phenomenal work! Thank you!

There are some weird prepended and appended docstrings from a parent method that I'd like to see added to this PR, which will improve other docstrings automatically.

The change log needs to be moved, per the release of 7.1.1, and described in https://icalendar.readthedocs.io/en/stable/contribute/index.html#change-log-entry-format.

Other than that, I pretty much had only grammar and style fixes.

Comment thread src/icalendar/cal/alarm.py Outdated
Comment thread src/icalendar/cal/alarm.py Outdated
Defined in :rfc:`9074`. Setting this property allows calendar clients to
dismiss or suppress an alarm across multiple devices. Once set to a value
greater than or equal to the alarm's computed trigger time, conforming clients
will not re-fire the alarm.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
will not re-fire the alarm.
will not refire the alarm.

>>> from icalendar import Alarm
>>> alarm = Alarm()
>>> alarm.ACKNOWLEDGED = datetime(2024, 1, 15, 10, 0, 0, tzinfo=timezone.utc)
>>> alarm.ACKNOWLEDGED # doctest: +ELLIPSIS
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline comments don't render. Is the comment necessary?

Suggested change
>>> alarm.ACKNOWLEDGED # doctest: +ELLIPSIS
>>> alarm.ACKNOWLEDGED

datetime.datetime(2024, 1, 15, 10, 0, tzinfo=...)

See also:
:attr:`TRIGGER` — the time at which the alarm fires.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:attr:`TRIGGER` the time at which the alarm fires.
:attr:`TRIGGER`, the time at which the alarm fires.

Comment on lines +648 to +649
Analogous to a file's modification timestamp. This property is optional;
when absent, :attr:`last_modified` falls back to :attr:`DTSTAMP`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Analogous to a file's modification timestamp. This property is optional;
when absent, :attr:`last_modified` falls back to :attr:`DTSTAMP`.
It's analogous to a file's modification timestamp. This property is optional.
When it's absent, :attr:`last_modified` falls back to :attr:`DTSTAMP`.

Analogous to a file's modification timestamp. This property is optional;
when absent, :attr:`last_modified` falls back to :attr:`DTSTAMP`.

Applicable to ``VEVENT``, ``VTODO``, ``VJOURNAL``, and ``VTIMEZONE``
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Applicable to ``VEVENT``, ``VTODO``, ``VJOURNAL``, and ``VTIMEZONE``
This property is applicable to ``VEVENT``, ``VTODO``, ``VJOURNAL``, and ``VTIMEZONE``

store.
"""The UTC datetime when this calendar component was first created, per :rfc:`5545#section-3.8.7.1`.

Records when the calendar user agent originally stored the component.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Records when the calendar user agent originally stored the component.
This property records when the calendar user agent originally stored the component.

"""The UTC datetime when this calendar component was first created, per :rfc:`5545#section-3.8.7.1`.

Records when the calendar user agent originally stored the component.
This property is optional; when absent, :attr:`created` falls back to
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This property is optional; when absent, :attr:`created` falls back to
This property is optional. When it's absent, :attr:`created` falls back to

The property can be specified once in "VEVENT",
"VTODO", or "VJOURNAL" calendar components. The value MUST be
specified as a date with UTC time.
Applicable to ``VEVENT``, ``VTODO``, and ``VJOURNAL`` components.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Applicable to ``VEVENT``, ``VTODO``, and ``VJOURNAL`` components.
This property is applicable to ``VEVENT``, ``VTODO``, and ``VJOURNAL`` components.

@stevepiercy
Copy link
Copy Markdown
Member

@ykd007 in addition to addressing my comments, would you please:

Please let us know if you need assistance with moving this forward. Thank you!

ykd007 added a commit to ykd007/ikalendar that referenced this pull request May 25, 2026
@ykd007
Copy link
Copy Markdown
Author

ykd007 commented May 25, 2026

Thank you for the detailed feedback, @stevepiercy!

I've pushed two commits addressing your requests:

  1. Reverted CHANGES.rst to match upstream and added a towncrier news fragment at news/1385.documentation following the new format.
  2. Updated the single_utc_property prepended docstring to The {name} property with all values converted to a ``:class:~datetime.datetime in UTC. with proper blank-line separation.
  3. Updated the ACKNOWLEDGED docs first line to match your suggested text.
  4. Cleaned up the p_doc boilerplate in create_single_property to use del/None literals and restructured the parameter note.

Please let me know if anything else needs attention!

@stevepiercy
Copy link
Copy Markdown
Member

@ykd007 GitHub shows that there are merge conflicts that need to be resolved. Would you please take care of that? Then I can complete a review and proceed. Thank you!

Hermes Agent and others added 3 commits May 25, 2026 10:53
…lective#1244)

Rewrote RFC-verbatim docstrings for the following properties:

alarm.py:
- Alarm.ACKNOWLEDGED: replaced RFC 9074 Purpose/Description wall-of-text
  with a concise summary, Return, Example and See also sections.
- Alarm.TRIGGER: replaced RFC 5545 Purpose/Value-Type verbatim with a
  clear summary, type info, Example and See also.

component.py:
- Component.DTSTAMP / stamp: replaced 'RFC 5545: Conformance/Description'
  block with a Pythonic summary, context note, Example and See also.
- Component.LAST_MODIFIED: same pattern - Pythonic summary, Example, See also.
- Component.CREATED: same pattern - Pythonic summary, Example, See also.

calendar.py:
- Calendar.calendar_name: removed Property Parameters/Conformance/Description
  RFC headings; added See also.
- Calendar.description: same cleanup.
- Calendar.color: removed Property Parameters/Conformance/Description RFC
  headings; added CSS3 link reference and See also.
- Calendar.prodid: replaced Conformance/Description verbatim with Pythonic
  summary, Example and See also.
- Calendar.version: replaced Purpose RFC text with Pythonic summary + Example.
- Calendar.calscale: replaced Conformance/Description/Compatibility with
  Pythonic summary, RFC 7529 note, Example and See also.
- Calendar.method: replaced Description RFC block with Pythonic summary,
  iTIP/RFC 5546 reference, Example and See also.
- Calendar.refresh_interval: replaced Conformance/Description headings with
  Pythonic summary, updated Raises, added Example and See also.
@ykd007 ykd007 force-pushed the fix/issue-1244-pythonic-docstrings branch from 86791a0 to ea0101f Compare May 25, 2026 05:25
@ykd007
Copy link
Copy Markdown
Author

ykd007 commented May 25, 2026

Hi @stevepiercy — I've rebased on current main and resolved all merge conflicts. The branch is clean now. Also tightened a few things during the rebase:

  • Removed the duplicate :rfc:9074`` reference in the ACKNOWLEDGED docstring
  • Fixed the invalid :rfc:5545#section-3.8.6.3 role (section anchors aren't supported) to just `:rfc:`5545
  • Cleaned up the p_doc boilerplate in create_single_property to plain prose instead of a misapplied Parameters section

Should be conflict-free now — please let me know if anything else needs attention.

@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 26384814733

Coverage at 97.784% (no base build to compare)

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 12717
Covered Lines: 12440
Line Coverage: 97.82%
Relevant Branches: 778
Covered Branches: 756
Branch Coverage: 97.17%
Branches in Coverage %: Yes
Coverage Strength: 2.93 hits per line

💛 - Coveralls

@ykd007 ykd007 force-pushed the fix/issue-1244-pythonic-docstrings branch from ea0101f to 7c55781 Compare May 25, 2026 06:44
Copy link
Copy Markdown
Member

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started to review your changes, but it appears you missed about two dozen of my suggestions and comments. I stopped. Would you please take care of them all? Thank you!

Comment thread src/icalendar/attr.py
@@ -960,10 +960,9 @@ def p_del(self: Component):
{doc}

Accepted values: {", ".join(t.__name__ for t in value_type)}.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Accepted values: {", ".join(t.__name__ for t in value_type)}.
Parameters:
{", ".join(t.__name__ for t in value_type)}.

Comment thread src/icalendar/attr.py
Comment on lines +963 to +965
Raises :exc:`~icalendar.error.InvalidCalendar` if the value is invalid.
Returns ``None`` if absent.
You can also delete the value with ``del`` or by setting it to ``None``.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the docstring format.

Suggested change
Raises :exc:`~icalendar.error.InvalidCalendar` if the value is invalid.
Returns ``None`` if absent.
You can also delete the value with ``del`` or by setting it to ``None``.
Raises:
InvalidCalendar: if the attribute has invalid values.
Returns:
A :class:`~datetime.datetime` or :class:`~datetime.timedelta` if the
value is valid. ``None`` if the value is absent.
"""

Comment thread src/icalendar/attr.py
Comment on lines 960 to 961
{doc}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{doc}
{doc}
You can also delete the value with ``del`` or by setting it to ``None``.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants