Skip to content

Add type hints and docstring to Timezone._make_unique_tzname - #1878

Open
zexi25 wants to merge 18 commits into
PyCalendar:mainfrom
zexi25:patch-1
Open

zexi25 wants to merge 18 commits into
PyCalendar:mainfrom
zexi25:patch-1

Conversation

@zexi25

@zexi25 zexi25 commented Oct 5, 2026 •

Copy link
Copy Markdown

Linked issue

Description

Update docstring of _make_unique_tzname(), to_tz(), from_tzid()in src/icalendar/cal/timezone.py to include:

  • a one-line summary
  • extended description
  • Parameters section
  • Returns sections

I used AI (Claude Opus 5.5) to review my draft and suggest wording and examples. I verified the final docstring and examples against the implementation.

Checklist

  • I added a change log entry, following the instructions in all subsections under Change log requirements.
  • 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

Documentation preview:
https://icalendar--1878.org.readthedocs.build/en/1878/_modules/icalendar/cal/timezone.html

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Profile summary:

GitHub user: zexi25
🟡 Some 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 zexi25`.
Full profile
GitHub user: zexi25
🟡 Some concerns found with user's profile.
   🟢 Account age: 9 months
   🟡 Profile information:
        name: Zexi
      Empty fields: company, blog, location, email, bio

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

   🟢 0 of 1 external PRs merged in the last 21 days.
   🟢 0 of 1 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.

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions github-actions Bot added the ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding. label Oct 5, 2026
@read-the-docs-community

read-the-docs-community Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Documentation build overview

📚 icalendar | 🛠️ Build #34988071 | 📁 Comparing adceb43 against latest (1a12179)

  🔍 Preview build  

1 file changed
± _modules/icalendar/cal/timezone.html

@coveralls-official

coveralls-official Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Coverage Status

coverage: 97.822%. remained the same — zexi25:patch-1 into collective:main

@niccokunzmann niccokunzmann left a comment

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.

Thanks for the PR! There is one suggestion.

In the docstrings, I would like to say what people can expect but not how it is done.

Comment thread src/icalendar/cal/timezone.py Resolved
zexi25 and others added 2 commits October 7, 2026 01:34
Co-authored-by: Nicco Kunzmann <niccokunzmann@rambler.ru>
@zexi25
zexi25 requested a review from niccokunzmann October 7, 2026 08:50
@faresrafat3

Copy link
Copy Markdown

Reviewed this locally on the PR branch.

The change. Docs + type hints: converts _make_unique_tzname docstring to Google style, adds type hints (str, set[str], -> str), and adds a doctest. No runtime behavior change.

Verification. Doctest passes. No issues.

Nothing to fix. Mergeable once CI is green.

niccokunzmann
niccokunzmann previously approved these changes Oct 28, 2026

@niccokunzmann niccokunzmann left a comment

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.

I think, the changelog is correct as we cannot link this.

The documentation also looks alright.

@niccokunzmann

Copy link
Copy Markdown
Member

@zexi25 For this to merge, you will need to rename the changelog file and add a number after it... Someone else also added to this issue.

@zexi25

zexi25 commented Oct 28, 2026

Copy link
Copy Markdown
Author

@niccokunzmann Appreciate your feedback! I've renamed the changelog with a number suffix. Sorry for accidentally dismissing your review.

@zexi25
zexi25 requested a review from niccokunzmann October 29, 2026 19:16

@stevepiercy stevepiercy left a comment

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.

This is a nice improvement. Thank you! I made a suggestion to use reStructuredText cross-references that will link to the objects you updated where they're present, and added a few more suggestions that were in the original code that needed to be cleaned up. Would you please take care? Thank you!

Comment thread news/1072.documentation.2 Resolved
Comment thread src/icalendar/cal/timezone.py Resolved
Comment thread src/icalendar/cal/timezone.py Resolved
Comment thread src/icalendar/cal/timezone.py Resolved
zexi25 and others added 6 commits November 12, 2026 16:47
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>
Co-authored-by: Steve Piercy <web@stevepiercy.com>

This branch has not been deployed

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

Labels

ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants