docs: Fix broken cross-references to Python objects across the codebase - #1679
docs: Fix broken cross-references to Python objects across the codebase#1679morning-verlu wants to merge 2 commits into
Conversation
|
This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments. |
|
Profile summary: Full profile |
|
@morning-verlu would you please address the failing CI checks? We don't review PRs until they pass all these checks. |
stevepiercy
left a comment
There was a problem hiding this comment.
Thanks for you contribution! It fixes several things, but I found a couple of instances that need special attention. Would you please take care?
| @@ -0,0 +1 @@ | |||
| Fixed broken Sphinx cross-references to Python objects throughout the codebase, including ``~error.``, ``~icalendar.Component.``, and unqualified class/method references. | |||
There was a problem hiding this comment.
Note that ~error.InvalidCalendar under a Raises section heading in a docstring is perfectly valid syntax and is not "broken." See https://icalendar.readthedocs.io/en/latest/reference/api/icalendar.cal.alarm.html#icalendar.cal.alarm.Alarm.new. This is a standardization.
However, AI is not intelligent despite its name, and will continue to force the issue, and I reckon that it's slightly more helpful to the reader of source code, so I'll go along with it.
Adjusted the news fragment accordingly, and give yourself credit.
| Fixed broken Sphinx cross-references to Python objects throughout the codebase, including ``~error.``, ``~icalendar.Component.``, and unqualified class/method references. | |
| Fixed broken and standardized Sphinx cross-references to Python objects throughout the code base, including ``~icalendar.Component.``, unqualified class and method references, and ``~error.``. @morning-verlu |
There was a problem hiding this comment.
Also please rename this file to 1158.documentation.2 to automatically generate a link to the issue when the news fragments are compiled into the change log. Thank you!
| Parameters: | ||
| categories: The :attr:`categories` of the Available component. | ||
| comments: The :attr:`~icalendar.Component.comments` of the Available | ||
| comments: The :attr:`~icalendar.cal.component.Component.comments` of the Available |
There was a problem hiding this comment.
All these parameters look much better now. Thank you!
|
|
||
| Returns: | ||
| A new :class:`vBoolean` instance with the supplied property parameters. | ||
| A new :class:`~icalendar.prop.boolean.vBoolean` instance with the supplied property parameters. |
There was a problem hiding this comment.
This is another example that standardizes the working link. Both forms are valid. See https://icalendar.readthedocs.io/en/latest/reference/api/icalendar.prop.boolean.html.
| """Parse a CATEGORIES value from iCalendar format. | ||
|
|
||
| This helper is normally called by :meth:`Component.from_ical`, which | ||
| This helper is normally called by :meth:`~icalendar.cal.component.Component.from_ical`, which |
There was a problem hiding this comment.
The original syntax didn't link, so well-spotted! However, here we want to preserve the display of the last two segments of the dotted path to disambiguate which from_ical is called throughout the code base.
| This helper is normally called by :meth:`~icalendar.cal.component.Component.from_ical`, which | |
| This helper is normally called by :meth:`Component.from_ical <icalendar.cal.component.Component.from_ical>`, which |
| Parameters: | ||
| ical: A list of category strings (preferred, as provided by | ||
| :meth:`Component.from_ical`), or a single comma-separated | ||
| :meth:`~icalendar.cal.component.Component.from_ical`), or a single comma-separated |
There was a problem hiding this comment.
| :meth:`~icalendar.cal.component.Component.from_ical`), or a single comma-separated | |
| :meth:`Component.from_ical <icalendar.cal.component.Component.from_ical>`), or a single comma-separated |
| ``"date-time"`` for ``DTSTART`` or ``"duration"`` for ``TRIGGER``. | ||
| This is the value type a property uses when no explicit ``VALUE`` | ||
| parameter is given, so it tells :func:`Component.from_jcal` whether a | ||
| parameter is given, so it tells :func:`~icalendar.cal.component.Component.from_jcal` whether a |
There was a problem hiding this comment.
This is actually a method, being under a class.
| parameter is given, so it tells :func:`~icalendar.cal.component.Component.from_jcal` whether a | |
| parameter is given, so it tells :meth:`Component.from_ical <icalendar.cal.component.Component.from_ical>` whether a |
|
@morning-verlu would you please resolve the merge conflicts as well? We can't merge until that's done. Thank you! |
niccokunzmann
left a comment
There was a problem hiding this comment.
Ready for merge after @stevepiercy's suggestions. Since this covers a lot of places, let's get it merged quickly to avoid spending time on merge conflicts.
Fix unqualified and short-form Sphinx cross-references throughout the codebase so that documentation links resolve correctly: - Replace ~error.X with ~icalendar.error.X (JCalParsingError, InvalidCalendar, IncompleteComponent) in 28 source files - Replace ~icalendar.Component.attr with ~icalendar.cal.component.Component.attr in 32 references - Qualify :class:BUSYTYPE and :class:FBTYPE in enums.py - Qualify :class:vBoolean in boolean.py - Qualify :meth:Component.from_ical in categories.py - Qualify :func:Component.from_jcal in factory.py and component.py Contributes to collective#1158
c3e9645 to
7057ca9
Compare
|
Rebased onto the current main branch and resolved the merge conflicts. The refreshed documentation, ruff, test, fuzzing, and package checks are all passing now. |
|
@morning-verlu Please let us know when we should have another look! Are you still working on this or should we review? You can also let us know in the comments above if they have been addressed or if you have a question or idea. |
|
@morning-verlu would you please address my review? See #1679 (review) |
Summary
This PR fixes broken Sphinx cross-references to Python objects throughout the source code, addressing issue #1158.
Changes
Short-form
~error.references →~icalendar.error.Fixed
~error.JCalParsingError,~error.InvalidCalendar, and~error.IncompleteComponentreferences to use the fully qualified~icalendar.error.path in 28 files acrosssrc/icalendar/.Re-exported
~icalendar.Component.references →~icalendar.cal.component.Component.Fixed 32 references that used the re-exported
icalendar.Componentpath instead of the actual module pathicalendar.cal.component.Component. Affected files includeevent.py,journal.py,todo.py,available.py, and others.Unqualified class/method references
:class:BUSYTYPE→ `:class:`~icalendar.enums.BUSYTYPEinenums.py:class:FBTYPE→ `:class:`~icalendar.enums.FBTYPEinenums.py:class:vBoolean→ `:class:`~icalendar.prop.boolean.vBooleaninboolean.py:meth:Component.from_ical→ `:meth:`~icalendar.cal.component.Component.from_icalincategories.py:func:Component.from_jcal→ `:func:`~icalendar.cal.component.Component.from_jcalinfactory.pyandcomponent.pyVerification
All references now use fully qualified names so Sphinx can resolve them correctly in the generated documentation.
Contributes to #1158.