Refactor iCal exporter by KJhellico · Pull Request #2977 · vacanza/holidays

KJhellico · 2025-10-02T10:27:34Z

Proposed change

Speed up date operations by using _timedelta instead of datetime.timedelta.

Type of change

New country/market holidays support (thank you!)
Supported country/market holidays update (calendar discrepancy fix, localization)
Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
Dependency update (version deprecation/pin/upgrade)
Bugfix (non-breaking change which fixes an issue)
Breaking change (a code change causing existing functionality to break)
New feature (new holidays functionality in general)

Checklist

I've read and followed the contributing guidelines.
I've run make check locally; all checks and tests passed.

coderabbitai · 2025-10-02T10:27:40Z

Summary by CodeRabbit

Performance
- Improved efficiency of iCal generation, reducing processing overhead for large holiday ranges.
Refactor
- Simplified date handling and streamlined iteration logic for clearer, more maintainable code without changing output.

Walkthrough

Refactors holidays/ical.py to use a centralized _timedelta helper, adds its import, and precomputes the dates list length for reuse in loops. Updates continuity checks to call _timedelta instead of datetime.timedelta and replaces repeated len() calls with a stored variable.

Changes

Cohort / File(s)	Summary of changes
iCal date range logic refactor `holidays/ical.py`	Import _timedelta from holidays.calendars.gregorian; replace datetime.timedelta usage with _timedelta; precompute length of sorted_dates as n and use in loops; adjust continuity check to _timedelta(sorted_dates[i], days); minor loop structure update to reuse n.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

PPsyrius
arkid15r

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title “Refactor iCal exporter” succinctly summarizes the primary change in this PR, which optimizes the iCal generation logic by replacing datetime.timedelta calls with a centralized helper and improving loop efficiency, and it clearly reflects the main focus of the changeset.
Description Check	✅ Passed	The description directly states that date operations are sped up by using `_timedelta` instead of `datetime.timedelta`, which aligns exactly with the implemented changes and clearly communicates the intent of the refactor.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4c369e and bdb67d9.

📒 Files selected for processing (1)

holidays/ical.py (2 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-04-03T16:58:27.175Z

Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.

Applied to files:

holidays/ical.py

🧬 Code graph analysis (1)

holidays/ical.py (2)

holidays/calendars/gregorian.py (1)

_timedelta (37-42)

holidays/holiday_base.py (1)

get_list (974-989)

⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Test Python 3.13 on windows-latest

🔇 Additional comments (4)

holidays/ical.py (4)

15-15: Import changes look good.

The removal of timedelta from the datetime import and the addition of _timedelta from holidays.calendars.gregorian align correctly with the refactor. The helper function is properly imported and ready to use.

Also applies to: 18-18

189-189: Smart optimization—precomputing the list length.

Caching len(sorted_dates) as n eliminates redundant calls in the loop. Since sorted_dates doesn't change during iteration, this is both safe and efficient.

191-191: Loop bounds updated correctly with cached length.

Both the outer loop condition and the inner continuity check now use n instead of calling len(sorted_dates) repeatedly. Logic is unchanged, just more efficient.

Also applies to: 198-198

199-199: Refactor to _timedelta is correct and efficient.

Replacing sorted_dates[i] + timedelta(days=days) with _timedelta(sorted_dates[i], days) maintains the same logic while leveraging ordinal arithmetic for better performance. The continuity check remains accurate across all calendar boundaries.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Ruff (0.13.2)

holidays/ical.py

�[1;31mruff failed�[0m
�[1mCause:�[0m Failed to load configuration /ruff.toml
�[1mCause:�[0m Failed to parse /ruff.toml
�[1mCause:�[0m TOML parse error at line 26, column 3
|
26 | "RSE100", # Use of assert detected
| ^^^^^^^^
Unknown rule selector: RSE100

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

sonarqubecloud · 2025-10-02T10:28:35Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

codecov · 2025-10-02T10:29:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (a4c369e) to head (bdb67d9).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff            @@
##               dev     #2977   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          301       301           
  Lines        17962     17964    +2     
  Branches      2319      2319           
=========================================
+ Hits         17962     17964    +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

PPsyrius

LGTM 👍

Good catch, not sure how we missed that during the initial PR review, but at least it's fixed now

Refactor iCal exporter

bdb67d9

KJhellico requested review from PPsyrius and arkid15r as code owners October 2, 2025 10:27

PPsyrius approved these changes Oct 2, 2025

View reviewed changes

arkid15r added this pull request to the merge queue Oct 2, 2025

Merged via the queue into vacanza:dev with commit 331e09f Oct 2, 2025
36 checks passed

KJhellico deleted the ref-ical branch October 3, 2025 14:40

arkid15r mentioned this pull request Oct 6, 2025

v0.82 #2988

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor iCal exporter#2977

Refactor iCal exporter#2977
arkid15r merged 1 commit into
vacanza:devfrom
KJhellico:ref-ical

KJhellico commented Oct 2, 2025

Uh oh!

coderabbitai Bot commented Oct 2, 2025 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented Oct 2, 2025

Uh oh!

codecov Bot commented Oct 2, 2025 •

edited

Loading

Uh oh!

PPsyrius left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

KJhellico commented Oct 2, 2025

Proposed change

Type of change

Checklist

Uh oh!

coderabbitai Bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

sonarqubecloud Bot commented Oct 2, 2025

Quality Gate passed

Uh oh!

codecov Bot commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PPsyrius left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Oct 2, 2025 •

edited

Loading

codecov Bot commented Oct 2, 2025 •

edited

Loading