Add `ChristianHolidays::_add_saint_martins_day` method by PPsyrius · Pull Request #3023 · vacanza/holidays

PPsyrius · 2025-10-28T02:44:37Z

Proposed change

I initially planned to include this in my PR for #3022, but since @KJhellico has also been working on that, I’m splitting this change into a separate PR for better atomicity.

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-28T02:45:00Z

Summary by CodeRabbit

Refactor
- Introduced a dedicated method for handling Saint Martin's Day (November 11) across the holiday system, improving code organization and maintainability for multiple countries including Italy, Sint Maarten, and Austria.

Walkthrough

Introduces a dedicated _add_saint_martins_day method in the ChristianHolidays group and replaces direct _add_holiday_nov_11 calls with _add_saint_martins_day in Italy, Sint Maarten, and Austria country definitions.

Changes

Cohort / File(s)	Summary
New method `holidays/groups/christian.py`	Adds `_add_saint_martins_day(self, name) -> date` which delegates to existing `_add_holiday_nov_11(name)` to register November 11 (Saint Martin of Tours Day).
Method adoption `holidays/countries/italy.py`, `holidays/countries/sint_maarten.py`, `holidays/countries/austria.py`	Replace direct `_add_holiday_nov_11(...)` calls with `_add_saint_martins_day(...)` for the November 11 Saint Martin holiday.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Small, consistent refactor across a few files.
Areas to spot-check: holidays/groups/christian.py new method docstring/typing and each country file's call sites for correct parameters/translation usage.

Suggested labels

test

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 33.33% 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 PR title "Add `ChristianHolidays::_add_saint_martins_day` method" directly and accurately describes the primary change introduced in this changeset. The raw summary confirms that the main modification is the introduction of a new public method `_add_saint_martins_day` in the `ChristianHolidays` class, which is then utilized across multiple country files. The title is specific, clear, and concise, clearly communicating the core intent of the pull request.
Description Check	✅ Passed	The PR description is related to the changeset and provides meaningful context for the change. It explains that this refactoring effort was originally planned as part of PR #3022 but is being split out separately for better atomicity, and it correctly identifies the change as a code quality improvement. While the description doesn't detail every technical aspect, it provides sufficient context to understand the motivation and purpose of introducing the new method.

✨ 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 5d42100 and 88a8254.

📒 Files selected for processing (1)

holidays/countries/austria.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-06-18T10:26:50.180Z

Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/france.py:178-180
Timestamp: 2025-06-18T10:26:50.180Z
Learning: In the France holidays implementation, the _populate_subdiv_57_public_holidays() (Moselle) and _populate_subdiv_6ae_public_holidays() (Alsace) methods are functionally identical, both adding Good Friday (≥1893) and Saint Stephen's Day (≥1892) with the same legal references from August 16th, 1892. Therefore, for the deprecated "Alsace-Moselle" subdivision, calling either method produces the same result.

Applied to files:

holidays/countries/austria.py

📚 Learning: 2025-06-18T10:07:58.780Z

Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/french_southern_territories.py:41-44
Timestamp: 2025-06-18T10:07:58.780Z
Learning: Territorial holiday classes that inherit from parent countries (like HolidaysAX from Finland, HolidaysSJ from Norway, HolidaysTF from France) follow a standard pattern of silently overriding self.subdiv in their _populate_public_holidays() method without validation, as this ensures they always use the correct subdivision code for their territory regardless of user input.

Applied to files:

holidays/countries/austria.py

🧬 Code graph analysis (1)

holidays/countries/austria.py (1)

holidays/groups/christian.py (1)

_add_saint_martins_day (458-466)

⏰ 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.14 on windows-latest

🔇 Additional comments (1)

holidays/countries/austria.py (1)

127-127: Clean refactoring.

Replacing the generic _add_holiday_nov_11 with the semantic _add_saint_martins_day improves clarity and maintains consistency with other saints' day methods like _add_saint_josephs_day. Functionally equivalent, no issues.

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-28T02:46:42Z

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff            @@
##               dev     #3023   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          306       306           
  Lines        18062     18064    +2     
  Branches      2333      2333           
=========================================
+ Hits         18062     18064    +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.

arkid15r

LGTM 👍

KJhellico · 2025-10-28T16:41:15Z

Austria also has this holiday.

sonarqubecloud · 2025-10-28T16:56:06Z

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

KJhellico

LGTM.

Add ChristianHolidays's _add_saint_martins_day method

5d42100

PPsyrius requested review from KJhellico and arkid15r as code owners October 28, 2025 02:44

arkid15r enabled auto-merge October 28, 2025 04:04

arkid15r reviewed Oct 28, 2025

View reviewed changes

KJhellico changed the title ~~Add ChristianHolidays's _add_saint_martins_day method~~ Add ChristianHolidays::_add_saint_martins_day method Oct 28, 2025

Apply this to Austria's holidays as well

88a8254

PPsyrius requested a review from arkid15r October 28, 2025 16:54

KJhellico approved these changes Oct 28, 2025

View reviewed changes

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

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

PPsyrius deleted the _add_saint_martins_day branch October 28, 2025 17:31

arkid15r mentioned this pull request Nov 3, 2025

v0.84 #3036

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `ChristianHolidays::_add_saint_martins_day` method#3023

Add `ChristianHolidays::_add_saint_martins_day` method#3023
arkid15r merged 2 commits intovacanza:devfrom
PPsyrius:_add_saint_martins_day

PPsyrius commented Oct 28, 2025

Uh oh!

coderabbitai bot commented Oct 28, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 28, 2025 •

edited

Loading

Uh oh!

arkid15r left a comment

Uh oh!

KJhellico commented Oct 28, 2025

Uh oh!

sonarqubecloud bot commented Oct 28, 2025

Uh oh!

KJhellico left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

PPsyrius commented Oct 28, 2025

Proposed change

Type of change

Checklist

Uh oh!

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

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

arkid15r left a comment

Choose a reason for hiding this comment

Uh oh!

KJhellico commented Oct 28, 2025

Uh oh!

sonarqubecloud bot commented Oct 28, 2025

Quality Gate passed

Uh oh!

KJhellico 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 28, 2025 •

edited

Loading

codecov bot commented Oct 28, 2025 •

edited

Loading