Add `_add_observed`/`_move_holiday`'s `force_observed` support #2986

PPsyrius · 2025-10-06T07:55:14Z

Proposed change

Based on PR discussion in #2984.

In some ways, the bot is right. When working on some countries (in Latin America, I think), I also wondered if there was a difference between the following cases:
N) ... "Some holiday" - October 12, ...
...
P) the holidays specified in paragraph N are observed on the following Monday
and
N) ... "Some holiday" - the first Monday after October 12
It is logical to assume that in the second case, October 12 is simply a calculation base and should not be a holiday even when observed=False.

Perhaps we should add a parameter to _add_observed and _move_holiday that would allow holidays to be moved unconditionally?

Something like force=True or force_observed=True...

Add _add_observed/_move_holiday's force_observed support.
Add _move_holiday_forced( ... ) as shorthand alias for _move_holiday( ..., force_observed=True, show_observed_label=False).
Refactor TestFiji to the latest test case standards.
Add _add_saint_patricks_day for ChristianHolidays Holiday Group.

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-06T07:55:26Z

Summary by CodeRabbit

New Features
- Standardized Saint Patrick’s Day handling across Ireland, Northern Ireland (UK), Montserrat, and the US.
- New Zealand: added Queen Elizabeth II Memorial Day (2022) and expanded Matariki coverage.
- Improved observed-date logic to support forced moves to the nearest Monday where applicable.
Improvements
- Canada (NL), Fiji (Girmit Day), and multiple New Zealand regions now consistently shift certain holidays to Monday to avoid conflicts.
Tests
- Fiji test suite expanded and modernized with broader year coverage and clearer observed/non-observed checks.

Walkthrough

Adds forced-observed support to observed-holiday core, introduces a Saint Patrick’s Day helper, and updates multiple country modules (Canada, New Zealand, Fiji, Ireland, Montserrat, UK, US) and tests to use the new helpers and forced nearest‑Monday observation rules; removes some _get_nearest_monday helpers.

Changes

Cohort / File(s)	Summary of changes
Observed holiday core `holidays/observed_holiday_base.py`	Added `force_observed` to `_add_observed` and `_move_holiday`; added `_move_holiday_forced(dt, rule)` to force observation and suppress observed label; adjusted control flow for moved holidays.
Christian group helper `holidays/groups/christian.py`	Added `_add_saint_patricks_day(self, name)` delegating to `_add_holiday_mar_17`.
Saint Patrick’s Day call-sites `holidays/countries/ireland.py`, `holidays/countries/montserrat.py`, `holidays/countries/united_kingdom.py`, `holidays/countries/united_states.py`	Replaced `_add_holiday_mar_17(...)` calls with `_add_saint_patricks_day(...)` (no behavior changes aside from helper use).
Canada (NL) optional holidays `holidays/countries/canada.py`	Removed `_get_nearest_monday` and `Optional` import; narrowed gregorian imports; NL optional holidays now use `_move_holiday_forced(..., ALL_TO_NEAREST_MON)` (St. Patrick’s, St. George’s with 2010 special-case, Discovery Day, Orangemen’s Day).
Fiji Girmit Day + tests `holidays/countries/fiji.py`, `tests/countries/test_fiji.py`	Switched Girmit Day to `_move_holiday_forced(..., ALL_TO_NEAREST_MON)`; removed `show_observed_label=False`; tests refactored to use `full_range` (2016–2049), added `no_estimated_holidays` and `workday_holidays`, removed `PUBLIC` import, and broadened assertions.
New Zealand overhaul `holidays/countries/new_zealand.py`	Removed `_get_nearest_monday` and `Optional` import; replaced ad‑hoc nearest‑Monday logic with `_move_holiday_forced` and explicit observed rules; clarified historical names, added Matariki handling, subdivision-specific adjustments, and QEII Memorial Day (2022).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Update Aruba, Curaçao, Netherlands holidays: general code refactor #2677 — modifies/standardizes observed-holiday movement logic; closely related to force-observed additions.
Add Fiji holidays #2354 — touches Fiji observed-handling and Girmit Day behavior/tests.
Add Montserrat holidays #2714 — updates Saint Patrick’s Day call-sites similar to this PR.

Suggested reviewers

KJhellico
arkid15r

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly summarizes the primary change by stating that support for force_observed is being added to the `_add_observed` and `_move_holiday` methods, accurately reflecting the main update in the pull request.
Description Check	✅ Passed	The description outlines the purpose and scope of the changes, detailing the addition of `force_observed` support, the new alias method, test refactors, and the added helper, all of which directly relate to the pull request’s modifications.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

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 7667f01 and b773915.

📒 Files selected for processing (1)

holidays/observed_holiday_base.py (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:17:53.342Z
Learning: In the Saint Vincent and the Grenadines holidays implementation, New Year's Day is added without observed rules using `_add_new_years_day()` and should not include observed rule testing in its test method. Only holidays explicitly wrapped with `_add_observed()` have observed behavior.

🧬 Code graph analysis (1)

holidays/observed_holiday_base.py (6)

holidays/countries/angola.py (2)

_is_observed (60-63)

_add_observed (65-71)

holidays/countries/tonga.py (1)

_add_observed (96-99)

tests/test_holiday_base.py (1)

_add_observed (43-50)

holidays/countries/jersey.py (1)

_add_observed (63-69)

holidays/countries/israel.py (1)

_add_observed (52-55)

holidays/holiday_base.py (1)

pop (1166-1195)

⏰ 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). (3)

GitHub Check: Test build on windows-latest
GitHub Check: Test build on ubuntu-latest
GitHub Check: Test build on macos-latest

🔇 Additional comments (1)

holidays/observed_holiday_base.py (1)

159-159: Logic looks solid.

The condition correctly implements the bypass behavior: force_observed=True skips the early return and proceeds with observed logic regardless of self.observed or _is_observed(dt) status.

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.3)

holidays/observed_holiday_base.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.}

codecov · 2025-10-06T07:59:40Z

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff            @@
##               dev     #2986   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          305       305           
  Lines        17996     17998    +2     
  Branches      2321      2323    +2     
=========================================
+ Hits         17996     17998    +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.

KJhellico

Great refactoring! Especially Canada and New Zealand.

holidays/observed_holiday_base.py

Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com>

coderabbitai

Actionable comments posted: 3

📜 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 31278ed and 7667f01.

📒 Files selected for processing (1)

holidays/observed_holiday_base.py (3 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:17:53.342Z
Learning: In the Saint Vincent and the Grenadines holidays implementation, New Year's Day is added without observed rules using `_add_new_years_day()` and should not include observed rule testing in its test method. Only holidays explicitly wrapped with `_add_observed()` have observed behavior.

🧬 Code graph analysis (1)

holidays/observed_holiday_base.py (6)

holidays/countries/angola.py (2)

_is_observed (60-63)

_add_observed (65-71)

holidays/countries/tonga.py (1)

_add_observed (96-99)

tests/test_holiday_base.py (1)

_add_observed (43-50)

holidays/countries/jersey.py (1)

_add_observed (63-69)

holidays/countries/israel.py (1)

_add_observed (52-55)

holidays/holiday_base.py (1)

pop (1166-1195)

⏰ 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). (2)

GitHub Check: Build distribution
GitHub Check: Test docs build

🔇 Additional comments (2)

holidays/observed_holiday_base.py (2)

159-159: LGTM!

The control flow correctly implements the forced-observed logic, matching the past review suggestion. When force_observed=True, it bypasses the normal self.observed and _is_observed(dt) checks.

207-207: LGTM!

The parameter pass-through is correct and maintains consistency between _move_holiday and _add_observed.

holidays/observed_holiday_base.py

KJhellico

LGTM! 👍

sonarqubecloud · 2025-10-09T03:49:18Z

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

arkid15r

LGTM 👍

Smart ObservedRule application.

PPsyrius added 3 commits October 6, 2025 14:20

Add _add_observed/_move_holiday's force_observed support

e8cf57f

Add _add_saint_patricks_day method

e188e67

Last Cleanup

31278ed

PPsyrius requested review from KJhellico and arkid15r as code owners October 6, 2025 07:55

github-actions bot added the test label Oct 6, 2025

PPsyrius mentioned this pull request Oct 6, 2025

Update Switzerland holidays: add Zurich's HALF_DAY, OPTIONAL holidays, th l10n support #2984

Merged

9 tasks

Merge branch 'dev' into force_observed

168370b

KJhellico reviewed Oct 7, 2025

View reviewed changes

holidays/observed_holiday_base.py Outdated Show resolved Hide resolved

holidays/observed_holiday_base.py Outdated Show resolved Hide resolved

PPsyrius and others added 2 commits October 7, 2025 18:57

Code Review Cleanup

7667f01

Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com>

Another one

b773915

Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com>

coderabbitai bot reviewed Oct 7, 2025

View reviewed changes

holidays/observed_holiday_base.py Show resolved Hide resolved

holidays/observed_holiday_base.py Show resolved Hide resolved

holidays/observed_holiday_base.py Outdated Show resolved Hide resolved

KJhellico approved these changes Oct 7, 2025

View reviewed changes

PPsyrius added 2 commits October 8, 2025 12:40

Merge branch 'dev' into force_observed

951b13f

Merge branch 'dev' into force_observed

a6c665e

arkid15r reviewed Oct 9, 2025

View reviewed changes

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

Merged via the queue into vacanza:dev with commit bda7987 Oct 9, 2025
36 checks passed

coderabbitai bot mentioned this pull request Oct 10, 2025

Drop Python 3.9 support #2998

Draft

9 tasks

PPsyrius deleted the force_observed branch October 10, 2025 04:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add `_add_observed`/`_move_holiday`'s `force_observed` support #2986

Add `_add_observed`/`_move_holiday`'s `force_observed` support #2986

Uh oh!

PPsyrius commented Oct 6, 2025

Uh oh!

coderabbitai bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

KJhellico left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KJhellico left a comment

Uh oh!

sonarqubecloud bot commented Oct 9, 2025

Uh oh!

arkid15r left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Add _add_observed/_move_holiday's force_observed support #2986

Add _add_observed/_move_holiday's force_observed support #2986

Uh oh!

Conversation

PPsyrius commented Oct 6, 2025

Proposed change

Type of change

Checklist

Uh oh!

coderabbitai bot commented Oct 6, 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

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Oct 9, 2025

Quality Gate passed

Uh oh!

arkid15r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add `_add_observed`/`_move_holiday`'s `force_observed` support #2986

Add `_add_observed`/`_move_holiday`'s `force_observed` support #2986

coderabbitai bot commented Oct 6, 2025 •

edited

Loading

codecov bot commented Oct 6, 2025 •

edited

Loading