-
-
Notifications
You must be signed in to change notification settings - Fork 539
Add Sint Maarten holidays #2631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update introduces a new holiday provider for Sint Maarten (SX), including its country code, holiday definitions, translations, and comprehensive tests. Documentation and registry files are updated to reflect Sint Maarten support, and a new contributor is added. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (1)holidays/countries/__init__.py (1)
🔇 Additional comments (3)
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
holidays/locale/en_US/LC_MESSAGES/SX.po (1)
21-75
:⚠️ Potential issueEmpty translations in
msgstr
fieldsAll
msgstr
entries are empty, which will cause blank labels for holidays in the default en_US locale. Please populate eachmsgstr
with the correspondingmsgid
(e.g.,msgstr "New Year's Day"
) or remove the file so the code falls back to the original English strings.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
CONTRIBUTORS
(1 hunks)README.md
(2 hunks)holidays/countries/__init__.py
(1 hunks)holidays/countries/sint_maarten.py
(1 hunks)holidays/locale/en_US/LC_MESSAGES/SX.po
(1 hunks)holidays/registry.py
(1 hunks)tests/countries/test_sint_maarten.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_sint_maarten.py (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.
🧬 Code Graph Analysis (2)
holidays/countries/__init__.py (1)
holidays/countries/sint_maarten.py (3)
SintMaarten
(21-94)SX
(97-98)SXM
(101-102)
tests/countries/test_sint_maarten.py (2)
tests/common.py (7)
CommonCountryTests
(356-374)assertAliases
(121-130)assertNoHolidays
(292-294)assertHolidays
(228-230)assertNoHolidayName
(273-275)assertHolidayName
(195-199)assertNoHoliday
(244-246)holidays/countries/sint_maarten.py (3)
SintMaarten
(21-94)SX
(97-98)SXM
(101-102)
🪛 Pylint (3.3.7)
holidays/countries/sint_maarten.py
[convention] 26-26: Line too long (116/100)
(C0301)
[convention] 27-27: Line too long (113/100)
(C0301)
[convention] 28-28: Line too long (120/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 97-97: Missing class docstring
(C0115)
[convention] 101-101: Missing class docstring
(C0115)
tests/countries/test_sint_maarten.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSintMaarten.setUpClass' method
(W0221)
[convention] 25-25: Missing function or method docstring
(C0116)
[convention] 28-28: Missing function or method docstring
(C0116)
[convention] 31-31: Missing function or method docstring
(C0116)
[convention] 51-51: Missing function or method docstring
(C0116)
[convention] 61-61: Missing function or method docstring
(C0116)
[warning] 62-62: Redefining name 'TestSintMaarten' from outer scope (line 20)
(W0621)
[convention] 62-62: Missing class docstring
(C0115)
[convention] 83-83: Missing function or method docstring
(C0116)
[convention] 88-88: Missing function or method docstring
(C0116)
[convention] 105-105: Missing function or method docstring
(C0116)
[convention] 110-110: Missing function or method docstring
(C0116)
[convention] 115-115: Missing function or method docstring
(C0116)
[convention] 135-135: Missing function or method docstring
(C0116)
[convention] 142-142: Missing function or method docstring
(C0116)
[convention] 147-147: Missing function or method docstring
(C0116)
🔇 Additional comments (6)
CONTRIBUTORS (1)
121-121
: Contributor Added in Correct OrderThe new entry for Platon Supranovich is correctly placed between Piotr Staniów and Prateekshit Jaiswal, preserving the alphabetical order.
holidays/registry.py (1)
174-174
: Sint Maarten Registered in COUNTRY DictionaryThe entry
"sint_maarten": ("SintMaarten", "SX", "SXM")
has been added consistently with existing country definitions.README.md (2)
108-108
: Supported Country Count UpdatedThe count was correctly incremented from 186 to 187 to reflect the addition of Sint Maarten.
1207-1212
: Sint Maarten Row Added to Country TableThe
<tr>
entry for Sint Maarten with code SX, no subdivisions, defaulten_US
language, and no extra categories is formatted consistently with other rows.holidays/countries/__init__.py (1)
171-171
: Import for SintMaarten Properly Inserted and SortedThe line
from holidays.countries.sint_maarten import SintMaarten, SX, SXM
is correctly added after Singapore and before Slovakia, preserving the import order.tests/countries/test_sint_maarten.py (1)
23-23
: 🧹 Nitpick (assertive)
setUpClass
signature mismatch flagged by pylint is expected – suppress it locally.Overriding
setUpClass(cls, entity_cls, years=...)
is the established pattern insideCommonCountryTests
. Consider adding a# pylint: disable=arguments-differ
comment to silence future noise.⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2609 File: tests/countries/test_nauru.py:20-24 Timestamp: 2025-06-13T15:15:25.128Z Learning: In the vacanza/holidays test suite, overriding `setUpClass` is intentionally done with the single `cls` parameter (no *args/**kwargs), so signature-mismatch lint warnings are ignored project-wide.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few suggestions for the beginning.
default_language = "en_US" | ||
supported_languages = ("en_US",) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't have plans to add other languages (such as Dutch), it is reasonable to delete everything related to localization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we got the original law in Dutch language, so perhaps we should consider adding that as well 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was told that you should only add languages that are native to you, so I only did English.
country = "SX" | ||
default_language = "en_US" | ||
supported_languages = ("en_US",) | ||
start_year = 2010 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start_year = 2010 | |
# Sint Maarten became a constituent country on October 10, 2010. | |
start_year = 2011 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So start year should be the year after it became a country?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need first full year.
super().__init__(*args, **kwargs) | ||
|
||
def _populate_public_holidays(self): | ||
# New Year's Day |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# New Year's Day | |
# New Year's Day. |
And for all comments below.
# King's Day (April 27 or April 26 if 27 is a Sunday) | ||
if self._year >= 2014: | ||
if date(self._year, APR, 27).weekday() == 6: | ||
self._add_holiday(tr("King's Day"), date(self._year, APR, 26)) | ||
else: | ||
self._add_holiday(tr("King's Day"), date(self._year, APR, 27)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can see an example of this holiday implementation in Curaçao.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So like this?
King's Day.
name = ("King's Day")
dt = date(self._year, APR, 27 if self._year >= 2014 else 30)
if self._is_sunday(dt):
dt = _timedelta(dt, -1 if self._year >= 1980 else +1)
self._add_holiday(name, dt)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but in your case, you don't need to check for 1980.
# Note: Carnival Day date may vary; verify annually | ||
self._add_holiday(tr("Carnival Day"), date(self._year, APR, 30)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Note: Carnival Day date may vary; verify annually | ||
self._add_holiday(tr("Carnival Day"), date(self._year, APR, 30)) | ||
# Carnival Day. | ||
self._add_holiday_apr_30(tr("Carnival Day")) |
# Note: Carnival Day date may vary; verify annually | |
self._add_holiday(tr("Carnival Day"), date(self._year, APR, 30)) | |
self._add_observed( | |
# Carnival Day. | |
self._add_holiday_apr_30(tr("Carnivalsdag"), rule=SAT_TO_PREV_FRI + SUN_TO_NEXT_TUE | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start so far, though for test case I would recommend the recently merged test_nauru.py as a base 👍
<td>Sint Maarten</td> | ||
<td>SX</td> | ||
<td></td> | ||
<td><strong>en_US</strong></td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering that the original law is in Dutch and the annual list is in English (mostly to preserve the original "Labour Day" spelling that's not in en_US
)👀
<td><strong>en_US</strong></td> | |
<td>en_SX, en_US, <strong>nl</strong></td> |
* <https://web.archive.org/web/20250606191248/https://en.wikipedia.org/wiki/Sint_Maarten> | ||
* <https://web.archive.org/web/20250529071040/https://en.wikipedia.org/wiki/Public_holidays_in_Sint_Maarten> | ||
* <https://web.archive.org/web/20250425015159/https://www.qppstudio.net/public-holidays/sint_maarten.htm> | ||
* <https://web.archive.org/web/20250212071023/https://www.sintmaartengov.org/Pages/Public-Holiday-Schedule.aspx> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional sources here 👍
* <https://web.archive.org/web/20250212071023/https://www.sintmaartengov.org/Pages/Public-Holiday-Schedule.aspx> | |
* [AB 2012 no. 19](https://web.archive.org/web/20250615040244/https://www.sintmaartengov.org/Documents/Official%20Publications/AB%2019%20Lvo%20Dag%20van%20Bevrijding.pdf) | |
* [AB 2015 no. 24](https://web.archive.org/web/20250615035932/https://www.sintmaartengov.org/Documents/Official%20Publications/AB%2024%20Landsverordening%20Constitution%20Day.pdf) | |
* [2014-2023](https://web.archive.org/web/20230307083630/https://www.sintmaartengov.org/government/VSA/labour/Pages/Public-Holiday-Schedule.aspx) | |
* [2024-Present](https://web.archive.org/web/20250212071023/https://www.sintmaartengov.org/Pages/Public-Holiday-Schedule.aspx) |
default_language = "en_US" | ||
supported_languages = ("en_US",) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we got the original law in Dutch language, so perhaps we should consider adding that as well 👀
# Note: Carnival Day date may vary; verify annually | ||
self._add_holiday(tr("Carnival Day"), date(self._year, APR, 30)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to 2014-2026 data (see my wayback machine links above), if it falls on Saturday then it got moved back to Friday, but if it falls on Sunday, this is usually moved to Tuesday, May 2nd instead not to overlap with Labour Day 👀
# Note: Carnival Day date may vary; verify annually | |
self._add_holiday(tr("Carnival Day"), date(self._year, APR, 30)) | |
self._add_observed( | |
# Carnival Day. | |
self._add_holiday_apr_30(tr("Carnivalsdag"), rule=SAT_TO_PREV_FRI + SUN_TO_NEXT_TUE | |
) |
# Constitution Day | ||
if self._year >= 2010: | ||
constitution_day = date(self._year, OCT, 1) + timedelta( | ||
days=(7 - date(self._year, OCT, 1).weekday()) % 7 + 7 | ||
) | ||
self._add_holiday(tr("Constitution Day"), constitution_day) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Constitution Day | |
if self._year >= 2010: | |
constitution_day = date(self._year, OCT, 1) + timedelta( | |
days=(7 - date(self._year, OCT, 1).weekday()) % 7 + 7 | |
) | |
self._add_holiday(tr("Constitution Day"), constitution_day) | |
# Established on September 28th, 2015. | |
if self._year >= 2015: | |
# Constitution Day. | |
self._add_holiday_2nd_mon_of_oct(tr("Dag van de Constitutie")) |
This replaces "Koninkrijksdag" (Kingdom Day) on DEC, 15 in years prior (please implement that as well).
"Project-Id-Version: Holidays 0.75\n" | ||
"POT-Creation-Date: 2025-06-14 14:20-0400\n" | ||
"PO-Revision-Date: 2025-06-14 14:20-0400\n" | ||
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your contact info goes here 👍
# SOME DESCRIPTIVE TITLE | ||
# This file is distributed under the same license as the Holidays package. | ||
# FIRST AUTHOR <EMAIL@ADDRESS>, 2025. B41A | ||
#, fuzzy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the default language, you can leave xx_XX localization
suffix out (just #Sint Maarten holidays.
)👍
# SOME DESCRIPTIVE TITLE | |
# This file is distributed under the same license as the Holidays package. | |
# FIRST AUTHOR <EMAIL@ADDRESS>, 2025. | |
#, fuzzy | |
# holidays | |
# -------- | |
# A fast, efficient Python library for generating country, province and state | |
# specific sets of holidays on the fly. It aims to make determining whether a | |
# specific date is a holiday as fast and flexible as possible. | |
# | |
# Authors: Vacanza Team and individual contributors (see CONTRIBUTORS file) | |
# dr-prodigy <dr.prodigy.github@gmail.com> (c) 2017-2023 | |
# ryanss <ryanssdev@icloud.com> (c) 2014-2017 | |
# Website: https://github.com/vacanza/holidays | |
# License: MIT (see LICENSE file) | |
# | |
# Sint Maarten holidays en_US localization. | |
# |
# Emancipation Day | ||
if self._year >= 2010: | ||
self._add_holiday(tr("Emancipation Day"), date(self._year, JUL, 1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Emancipation Day | |
if self._year >= 2010: | |
self._add_holiday(tr("Emancipation Day"), date(self._year, JUL, 1)) | |
# Established on June 13th, 2012. | |
if self._year >= 2012: | |
# Emancipation Day. | |
self._add_observed(self._add_holiday_jul_1(tr("Dag van de Bevrijding"))) |
def __init__(self, *args, **kwargs): | ||
ChristianHolidays.__init__(self) | ||
InternationalHolidays.__init__(self) | ||
super().__init__(*args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This and self._add_observed( ... )
goes for any holidays that isn't always observed on weekdays
super().__init__(*args, **kwargs) | |
kwargs.setdefault("observed_rule", SUN_TO_NEXT_MON) | |
super().__init__(*args, **kwargs) |
def test_pre_2010_edge_cases(self): | ||
holidays = SintMaarten(years=2009) | ||
self.assertNotIn(date(2009, 7, 1), holidays) # No Emancipation Day | ||
self.assertNotIn(date(2009, 10, 12), holidays) # No Constitution Day | ||
self.assertNotIn(date(2009, 11, 1), holidays) # No All Saints' Day | ||
self.assertNotIn(date(2009, 11, 11), holidays) # No Sint Maarten Day | ||
self.assertNotIn(date(2009, 4, 27), holidays) # No King's Day | ||
self.assertNotIn(date(2009, 4, 26), holidays) # No King's Day (Sunday case) | ||
self.assertNotIn(date(2009, 4, 30), holidays) # No Carnival Day | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already handled in test_no_holidays
def test_pre_2010_edge_cases(self): | |
holidays = SintMaarten(years=2009) | |
self.assertNotIn(date(2009, 7, 1), holidays) # No Emancipation Day | |
self.assertNotIn(date(2009, 10, 12), holidays) # No Constitution Day | |
self.assertNotIn(date(2009, 11, 1), holidays) # No All Saints' Day | |
self.assertNotIn(date(2009, 11, 11), holidays) # No Sint Maarten Day | |
self.assertNotIn(date(2009, 4, 27), holidays) # No King's Day | |
self.assertNotIn(date(2009, 4, 26), holidays) # No King's Day (Sunday case) | |
self.assertNotIn(date(2009, 4, 30), holidays) # No Carnival Day |
Proposed change
Adds support for Sint Maarten (SX) public holidays with full test coverage.
Resolves #2475
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green