Merge constructor docstrings into class docstrings by felix-windsor · Pull Request #1622 · collective/icalendar

felix-windsor · 2026-08-01T03:33:05Z

Linked issue

Closes Merge __init__ or __new__ docstrings into their class's docstring #1620

Description

Moves every __init__ and __new__ docstring in the package into its containing class docstring so API documentation has a single source of class-level content.

This also:

configures Sphinx with autoclass_content = "class";
preserves and organizes existing parameter, example, and exception documentation;
adds an AST regression test that reports any constructor docstrings added later;
adds the required documentation news fragment.

Responsible AI disclosure: I used OpenAI Codex (GPT-5) to inventory and migrate the docstrings, draft the AST regression test, update the Sphinx configuration and news entry, and review validation output. I reviewed the complete diff and validated the changes locally.

Checklist

I added a change log entry, following the instructions in Change log entry format.
I followed icalendar project guidelines for Artificial intelligence policy and disclosed my Responsible AI use in my commit messages, if applicable.
I added or updated tests, if applicable.
I ran and ensured all tests pass locally by following Run tests.
I added or edited documentation as necessary, both as docstrings to be rendered in the API documentation and narrative documentation, following the Style guide.

Additional information

Local validation:

uv run pytest -q: 17,888 passed, 28 skipped, 529 xfailed.
make test: CPython 3.10, 3.11, 3.12, 3.13, and 3.14 passed; the nopytz environment passed.
CARGO_NET_GIT_FETCH_WITH_CLI=true uv run tox -e pypy3: passed with 97% coverage.
Strict make html Sphinx build: passed with warnings treated as errors.
uv run ruff format --check, uv run ruff check, and git diff --check: passed.

The combined docs tox run completed HTML, doctest, and Vale successfully; its link-check phase only reported two pre-existing external-site responses unrelated to this change: a 404 from the sphinx-reredirects documentation URL and a 403 from a Stack Overflow timezone link.

AI disclosure: I used OpenAI Codex (GPT-5) to inventory all __init__ and __new__ docstrings, migrate and merge their content into class docstrings, add the AST regression test, update the Sphinx configuration and news entry, and review local test and documentation results. I reviewed and validated the changes.

github-actions · 2026-08-01T03:33:20Z

This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments.

github-actions · 2026-08-01T03:33:20Z

Profile summary:

GitHub user: felix-windsor
🟢 No concerns found with user's profile.
🟢 No concerns found with recent PR activity.
🟢 No concerns found with recent issue activity.

For a more detailed report, run `gh-profiler felix-windsor`.

Full profile

GitHub user: felix-windsor
🟢 No concerns found with user's profile.
   🟢 Account age: 2 years
   🟢 Profile information:
        name: Felix Wang
        company: University of Sydney
        blog: shengxi.xyz
        email: felixwindsor3344@gmail.com
        bio: You must realize a grand world awaits your exploration, far beyond the known.
      Empty fields: location

🟢 No concerns found with recent PR activity.
   3 PRs opened in the last 21 days.
      0 opened against repos the user owns.
      0 opened against repos in publicly associated orgs.
      3 opened against external repos.

   🟢 0 of 3 external PRs merged in the last 21 days.
   🟢 0 of 3 external PRs closed without merging in the last 21 days.

🟢 No concerns found with recent issue activity.
   🟢 No new issues opened in the last 21 days.

read-the-docs-community · 2026-08-01T03:34:30Z

Documentation build overview

📚 icalendar | 🛠️ Build #33862772 | 📁 Comparing ca3543f against latest (926bc02)

🔍 Preview build

45 files changed · ± 45 modified

± Modified

angatha · 2026-08-02T07:49:32Z

@krrishapatel please dont paste AI generated output like this. Keep the important parts.

Did you verify that the AI did its job correct?

I prefer not to do the suggestion since an __init__ method should only be within a class.

stevepiercy · 2026-08-02T08:02:36Z

I'm in the middle of a thorough review while in transit. Please don't merge until I have the chance to complete it.

stevepiercy

This is fantastic work. Thank you for your effort. Would you please review my suggestions, and take care? Thank you!

stevepiercy · 2026-08-01T06:44:48Z

    Component is the base object for calendar, Event and the other
    components defined in :rfc:`5545`. Normally you will not use this class


Suggested change

Component is the base object for calendar, Event and the other

components defined in :rfc:`5545`. Normally you will not use this class

Component is the base object for calendar, Event, and the other

components defined in :rfc:`5545`. Normally you won't use this class

stevepiercy · 2026-08-01T06:52:01Z


    All properties of the calendar component are parsed immediately.
    Subcomponents and their properties are parsed lazily.
+    A new calendar starts with no subcomponents.


This is a strange statement, and I don't think it's true in this context. The lazy module is special, in that it lazily parses subcomponents upon initialization. Perhaps @SashankBhamidi or @niccokunzmann can provide a more accurate description of what initialization actually does? Or maybe just remove this statement, as I think the existing description is accurate enough? My Python vocabulary is less proficient than theirs.

Yes, it's true, but it's equally true of a plain Calendar(). The __init__ sets InitialSubcomponentsStrategy, which transitions to LazySubcomponentsStrategy when super().__init__() sets subcomponents = []. The existing description already says what matters. I'd drop the sentence.

stevepiercy · 2026-08-01T06:57:44Z

+        component_factory: Required. The factory to use for creating components.
+        data: Required. The raw iCalendar data to parse, either as bytes or a
+            list of content lines.
+        types_factory: Required. The factory to use for creating property values.


Nice! Thanks for watching the latest update about sorting parameters and adding "Required."

One minor correction from the original.

Suggested change

component_factory: Required. The factory to use for creating components.

data: Required. The raw iCalendar data to parse, either as bytes or a

list of content lines.

types_factory: Required. The factory to use for creating property values.

component_factory: Required. The factory to use for creating components.

data: Required. The raw iCalendar data to parse, either as bytes, string,

or a list of content lines.

types_factory: Required. The factory to use for creating property values.

stevepiercy · 2026-08-01T07:00:33Z

+    Parameters:
+        name: Required. The name of the subcomponent.
+        parser: Required. The parser that holds the raw subcomponent data.


Another good improvement. ❤️

stevepiercy · 2026-08-02T09:17:55Z

+            computation.
+
+    Examples:
+        This example uses RFC 9074. One alarm is 30 minutes before the event and


Suggested change

This example uses RFC 9074. One alarm is 30 minutes before the event and

This example uses :rfc:`9074`. One alarm is 30 minutes before the event and

stevepiercy · 2026-08-02T09:18:27Z

-    >>> alarms.active[0].trigger  # this alarm triggers 15 minutes before 10:30
-    datetime.datetime(2021, 3, 2, 10, 15, tzinfo=ZoneInfo(key='America/New_York'))
-
    RFC 9074 specifies that alarms can also be triggered by proximity.


Suggested change

RFC 9074 specifies that alarms can also be triggered by proximity.

:rfc:`9074` specifies that alarms can also be triggered by proximity.

stevepiercy · 2026-08-02T09:21:33Z

+        component: An alarm, event, or to-do component with which to start the
+            computation.


Let's link these to the component class, and rewrap for ruff.

Suggested change

component: An alarm, event, or to-do component with which to start the

computation.

component: An :class:`~icalendar.cal.alarm.Alarm`, :class:`~icalendar.cal.event.Event`, or :class:`~icalendar.cal.todo.Todo` component with which to start the computation.

stevepiercy · 2026-08-02T09:22:36Z

+        **kwargs: Keyword arguments passed to :class:`~collections.OrderedDict`.

-        Example:
+    Examples:


I see only one example.

Suggested change

Examples:

Example:

stevepiercy · 2026-08-02T09:24:26Z

+        path: The location in the jCal structure where the error occurred.
+        parser: The parser class or its name where the error occurred.


Sort. path doesn't look required to me.

Suggested change

path: The location in the jCal structure where the error occurred.

parser: The parser class or its name where the error occurred.

parser: The parser class or its name where the error occurred.

path: The location in the jCal structure where the error occurred.

Solaris-star

Reviewed — this covers the same ground as my now-closed #1623, and does it more completely. A few observations:

autoclass_content = "class" in docs/conf.py — good catch, I missed this in my version. Without it Sphinx would still try to render __init__ docstrings if any slipped through.
Test placement: putting the AST check in the existing test_docstrings.py is better than a standalone file — keeps all docstring policy tests together.
Required. prefix: consistent with the convention @stevepiercy established in #1603. Nice.
One minor note: the test uses Path(icalendar.__file__).parent which resolves correctly for installed packages, but if someone runs tests from a source checkout without installing, the path still works since icalendar.__file__ points to the source tree. No issue, just confirming I checked.
The news fragment is present — my #1623 was missing that. Lesson learned.

Happy to help with any follow-up on this if needed.

SashankBhamidi

Thanks Felix. I ran the new test against main and confirmed it fails there.

Steve's suggestions are all still open. One more thing inline.

Procedural note: if any part of this was AI-assisted, please follow the Responsible AI use policy. Standard reminder.

SashankBhamidi · 2026-08-06T09:05:49Z

+        parent: The parent component to which the alarm refers.
+        snoozed_until: A datetime in UTC until which the alarm has been snoozed.


The parameter order in the docstring doesn't match the signature. The __init__ signature (line 60) has snoozed_until before parent, but the docstring has them reversed. Parameters should follow signature order.

Suggested change

parent: The parent component to which the alarm refers.

snoozed_until: A datetime in UTC until which the alarm has been snoozed.

snoozed_until: A datetime in UTC until which the alarm has been snoozed.

parent: The parent component to which the alarm refers.

Actually, we changed this in https://icalendar.readthedocs.io/en/latest/contribute/documentation/style-guide.html#docstring-structure. Alphabetically sorted within each group of required and optional parameters is correct.

SashankBhamidi · 2026-08-06T09:07:30Z


    All properties of the calendar component are parsed immediately.
    Subcomponents and their properties are parsed lazily.
+    A new calendar starts with no subcomponents.


Yes, it's true, but it's equally true of a plain Calendar(). The __init__ sets InitialSubcomponentsStrategy, which transitions to LazySubcomponentsStrategy when super().__init__() sets subcomponents = []. The existing description already says what matters. I'd drop the sentence.

SashankBhamidi · 2026-08-09T10:48:42Z

Hi @felix-windsor, could you take care of the conflicts please? Let us know if you need any support from our side.

niccokunzmann · 2026-08-17T10:39:41Z

Uh. Yes, that is a problem when creating a big PR on an active repository. @felix-windsor Your contribution is welcome and I hope to see it go through. If you like, create smaller PRs for the changes and they have less merge work in them.

felix-windsor requested review from SashankBhamidi, angatha, niccokunzmann and stevepiercy as code owners August 1, 2026 03:33

github-actions Bot added the ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding. label Aug 1, 2026

niccokunzmann mentioned this pull request Aug 1, 2026

Revised docstring style guide. #1621

Merged

5 tasks

stevepiercy mentioned this pull request Aug 1, 2026

docs: merge __init__/__new__ docstrings into class docstrings #1623

Closed

angatha reviewed Aug 2, 2026

View reviewed changes

Comment thread docs/conf.py

stevepiercy requested changes Aug 2, 2026

View reviewed changes

Solaris-star reviewed Aug 2, 2026

View reviewed changes

SashankBhamidi requested changes Aug 6, 2026

View reviewed changes

Solaris-star mentioned this pull request Aug 11, 2026

Merge __init__ or __new__ docstrings into their class's docstring #1620

Open

4 tasks

stevepiercy added the doc label Aug 23, 2026

		Component is the base object for calendar, Event and the other
		components defined in :rfc:`5545`. Normally you will not use this class

	This example uses RFC 9074. One alarm is 30 minutes before the event and
	This example uses :rfc:`9074`. One alarm is 30 minutes before the event and

	RFC 9074 specifies that alarms can also be triggered by proximity.
	:rfc:`9074` specifies that alarms can also be triggered by proximity.

		component: An alarm, event, or to-do component with which to start the
		computation.

	component: An alarm, event, or to-do component with which to start the
	computation.
	component: An :class:`~icalendar.cal.alarm.Alarm`, :class:`~icalendar.cal.event.Event`, or :class:`~icalendar.cal.todo.Todo` component with which to start the computation.

		path: The location in the jCal structure where the error occurred.
		parser: The parser class or its name where the error occurred.

		parent: The parent component to which the alarm refers.
		snoozed_until: A datetime in UTC until which the alarm has been snoozed.

Uh oh!

Uh oh!

Conversation

felix-windsor commented Aug 1, 2026

Linked issue

Description

Checklist

Additional information

Uh oh!

github-actions Bot commented Aug 1, 2026

Uh oh!

github-actions Bot commented Aug 1, 2026

Uh oh!

read-the-docs-community Bot commented Aug 1, 2026

Documentation build overview

Uh oh!

angatha commented Aug 2, 2026

Uh oh!

Uh oh!

stevepiercy commented Aug 2, 2026

Uh oh!

stevepiercy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Solaris-star left a comment

Choose a reason for hiding this comment

Uh oh!

SashankBhamidi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SashankBhamidi commented Aug 9, 2026

Uh oh!

niccokunzmann commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants