Replace quoted-list generated docstrings with static docstrings

[cakeni]

Linked issue

• See Replace generated docstrings with static docstrings #1650

Description

Moves quoted_list_parameter and its three callers from generated property docs to static Sphinx-visible attribute docstrings. Covers part of the seven-factory issue. AI use is disclosed in the commit and news fragment.

Checklist

• I added a change log entry, following the instructions in Change log entry format.
• I followed icalendar's 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

55 focused tests, targeted doctest, Ruff, format, diff, AST, and Towncrier pass; generated HTML includes them. Full Windows pytest hit an unrelated CRLF failure after 9,196 passes. Sphinx -W completed pages but hit 12 existing include warnings.

[github-actions]

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

[github-actions]

Profile summary:

GitHub user: cakeni
🔴 Significant 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 cakeni`.

Full profile

GitHub user: cakeni
🔴 Significant concerns found with user's profile.
   🟢 Account age: 1 year, 3 months
   🔴 No profile information provided.

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

   🟢 0 of 5 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]

Documentation build overview

📚 icalendar | 🛠️ Build #34296459 | 📁 Comparing 0720f85 against latest (f82f860)

  🔍 Preview build  

53 files changed · ± 53 modified

± Modified

• 404.html
• genindex.html
• _static/webpack-macros.html
• contribute/index.html
• how-to/attendees.html
• how-to/index.html
• how-to/upgrade.html
• reference/changelog.html
• _modules/icalendar/alarms.html
• _modules/icalendar/attr.html
• and 43 more...

[stevepiercy]

Although I thought this would be a good idea, I'm having second thoughts. Please see my comment.

[stevepiercy]

This has an interesting side effect, and I'm not sure whether it's better or worse. The docstring previously appeared in the context of:

• vCalAddress

It got moved to:

• icalendar.param

I think that makes it less useful because it loses its practical context. @niccokunzmann @cakeni what do you think? AI doesn't think.

Similar behavior occurred for DELEGATED_TO, but not MEMBER which appears to have been omitted by accident.

@niccokunzmann should MEMBER be treated the same as other CAL-ADDRESS parameters? This appears to be a bug.

[cakeni]

Thanks, I dug into this more carefully, and I agree that moving the docs to icalendar.param loses useful context.
Previously, quoted_list_parameter() passed the docstring into the property itself, so Sphinx could show it under vCalAddress.DELEGATED_FROM and vCalAddress.DELEGATED_TO. With the current static attribute docstrings, Sphinx instead associates them with icalendar.param.
I think the cleaner approach is to keep the descriptor implementation in icalendar.param, but put the static documentation on the concrete vCalAddress bindings. That keeps the docs in the practical context without duplicating them.
I also checked MEMBER: it was not missed by this PR. vCalAddress simply never exposed MEMBER, unlike DELEGATED_FROM and DELEGATED_TO, so this looks like a pre-existing inconsistency. Adding it would be a new public API, though, so I don't want to include that without maintainer confirmation.
Would you prefer that I keep #1651 limited to fixing the documentation location and handle MEMBER separately, or make all three consistent here?

[niccokunzmann]

Member should be treated the same as the others. That is a mistake.

The change can be in here or in a follow up PR.

[niccokunzmann]

This is the way, variables are documented. Nice feature.

[niccokunzmann]

This PR uses the same idea as the one in #1652.

Could you help me understand why you @cakeni and @floze-the-genius have chosen this path? Did you talk to each other?

The change that I see is that now, we document in a different place.
@stevepiercy That is an opportunity to change where the docs live and they can be more specific to the classes that these attributes reside in. However, we should probably check that this is consistent.

Before

property holds the docs

class1.PROP = class2.PROP = PROPERTY

class1.PROP and class2.PROP have the same docs

Now

attribute is documented

class1.PROP = class2.PROP = PROPERTY

class1.PROP has documentation distinct from class2.PROP

Evaluation

This is a documentation design decision. They use a 'newer' language feature. This increases the amount of documentation and possible duplication and inconsistency. However, it also allows tailoring the documentation of the property to the specific class that it lives in.
Moving the whole code base over is a big job (docstrings increase with the amount of occurrences in classes) but also a good first-timer issue.
We do not have to move the whole code base over.

What are your thoughts?

[cakeni]

This PR uses the same idea as the one in #1652.

Could you help me understand why you @cakeni and @floze-the-genius have chosen this path? Did you talk to each other?

The change that I see is that now, we document in a different place. @stevepiercy That is an opportunity to change where the docs live and they can be more specific to the classes that these attributes reside in. However, we should probably check that this is consistent.

Before

property holds the docs

class1.PROP = class2.PROP = PROPERTY

class1.PROP and class2.PROP have the same docs

Now

attribute is documented

class1.PROP = class2.PROP = PROPERTY

class1.PROP has documentation distinct from class2.PROP

Evaluation

This is a documentation design decision. They use a 'newer' language feature. This increases the amount of documentation and possible duplication and inconsistency. However, it also allows tailoring the documentation of the property to the specific class that it lives in. Moving the whole code base over is a big job (docstrings increase with the amount of occurrences in classes) but also a good first-timer issue. We do not have to move the whole code base over.

What are your thoughts?

I hadn’t seen #1652 and didn’t discuss this with the author. This just seemed like the simplest way to keep the code shared while putting the docs where people would expect them. I’d keep it limited to these parameters for now.

[stevepiercy]

A minor formatting tweak, then I approve. You helped us get unstuck. ❤️

Upon final approval and merge, we should close #1652, and update the checklist in the original issue #1650.

[stevepiercy]

Thank you for the changes.

[stevepiercy]

@niccokunzmann this LGTM. Have all your unresolved comments been addressed?

[coveralls]

coverage: 97.536% (+0.001%) from 97.535% — cakeni:codex/static-quoted-list-docstrings into collective:main