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 reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)tests/countries/test_sint_maarten.py (8)🪛 Pylint (3.3.7)tests/countries/test_sint_maarten.py[convention] 1-1: Missing module docstring (C0114) [convention] 19-19: Missing class docstring (C0115) [warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSintMaarten.setUpClass' method (W0221) [convention] 24-24: Missing function or method docstring (C0116) [convention] 27-27: Missing function or method docstring (C0116) [convention] 30-30: Missing function or method docstring (C0116) [convention] 33-33: Missing function or method docstring (C0116) [convention] 46-46: Missing function or method docstring (C0116) [convention] 59-59: Missing function or method docstring (C0116) [convention] 72-72: Missing function or method docstring (C0116) [convention] 95-95: Missing function or method docstring (C0116) [convention] 115-115: Missing function or method docstring (C0116) [convention] 120-120: Missing function or method docstring (C0116) [convention] 133-133: Missing function or method docstring (C0116) [convention] 146-146: Missing function or method docstring (C0116) [convention] 166-166: Missing function or method docstring (C0116) [convention] 180-180: Missing function or method docstring (C0116) [convention] 183-183: Missing function or method docstring (C0116) [convention] 194-194: Missing function or method docstring (C0116) [convention] 198-198: Missing function or method docstring (C0116) [convention] 217-217: Missing function or method docstring (C0116) [convention] 235-235: Missing function or method docstring (C0116) 🔇 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
holidays/locale/en_US/LC_MESSAGES/SX.po (1)
21-75:⚠️ Potential issueEmpty translations in
msgstrfieldsAll
msgstrentries are empty, which will cause blank labels for holidays in the default en_US locale. Please populate eachmsgstrwith 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_USlanguage, 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, SXMis correctly added after Singapore and before Slovakia, preserving the import order.tests/countries/test_sint_maarten.py (1)
23-23: 🧹 Nitpick (assertive)
setUpClasssignature 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-differcomment 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.
KJhellico
left a comment
KJhellico
left a comment
There was a problem hiding this comment.
A few suggestions for the beginning.
PPsyrius
left a comment
PPsyrius
left a comment
There was a problem hiding this comment.
Good start so far, though for test case I would recommend the recently merged test_nauru.py as a base 👍
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
KJhellico
left a comment
KJhellico
left a comment
There was a problem hiding this comment.
I think that's the last of my suggestions. :)
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>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/sint_maarten.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/sint_maarten.py (3)
Learnt from: KJhellico
PR: vacanza/holidays#2631
File: holidays/countries/sint_maarten.py:94-95
Timestamp: 2025-06-14T20:43:15.370Z
Learning: The `_add_*` helper methods in the holidays library (such as `_add_christmas_day_two()`, `_add_labor_day()`, etc.) don't have default holiday names, so the name parameter should always be explicitly specified when calling these methods.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:104-110
Timestamp: 2025-06-05T17:22:43.114Z
Learning: In the holidays library codebase, country alias classes (like SN, SEN for Senegal) follow a consistent convention of having no docstrings and just containing "pass". This pattern is used across all countries - examples include BR/BRA for Brazil, CA/CAN for Canada, DE/DEU for Germany, etc. Static analysis tools may flag missing docstrings, but the established codebase convention is to keep these alias classes minimal.
🪛 Pylint (3.3.7)
holidays/countries/sint_maarten.py
[convention] 30-30: Line too long (178/100)
(C0301)
[convention] 31-31: Line too long (187/100)
(C0301)
[convention] 32-32: Line too long (153/100)
(C0301)
[convention] 33-33: Line too long (134/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 113-113: Missing class docstring
(C0115)
[convention] 117-117: Missing class docstring
(C0115)
🔇 Additional comments (7)
holidays/countries/sint_maarten.py (7)
36-40: Well-structured country configuration.The country code, language settings, and start year are properly configured based on Sint Maarten's status as a constituent country from 2010.
42-46: Proper initialization with default observance rule.The constructor correctly initializes parent classes and sets the default observance rule to move Sunday holidays to Monday, which aligns with standard practices.
61-68: Excellent King's/Queen's Day implementation.The conditional logic properly handles the transition from Queen's Day (April 30) to King's Day (April 27) in 2014, with correct observance rules moving Sunday holidays to Saturday.
70-77: Solid Carnival Day implementation with proper observance.The implementation correctly handles Carnival Day with different rules before and after 2014, including proper weekend adjustments (Saturday to Friday, Sunday to Tuesday).
88-96: Proper handling of legislative changes.The conditional logic correctly implements Emancipation Day (from 2012) and Constitution Day (from 2015), while properly ending Kingdom Day in 2014. This matches the legislative timeline.
Also applies to: 101-104
113-118: Alias classes follow established conventions.The SX and SXM alias classes correctly follow the codebase pattern of minimal implementation without docstrings, consistent with other country implementations.
30-33: Line length exceeds project standards.The reference URLs exceed the 100-character line limit. Consider breaking them across multiple lines.
- * [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) + * [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)⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2671 File: holidays/countries/libya.py:25-25 Timestamp: 2025-06-22T21:34:15.750Z Learning: In the holidays project, Reference URLs in docstrings are kept as complete markdown links on single lines even when they exceed 100 characters, prioritizing reference readability over line length limits.
Signed-off-by: Platon Supranovich <psupra521@gmail.com>
|
4 participants
Proposed change
Adds support for Sint Maarten (SX) public holidays with full test coverage.
Resolves #2475
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green