Update Aruba, Curaçao, Netherlands holidays: general code refactor #2677
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes refactor Aruba, Curacao, and Netherlands holiday classes to use Changes
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)tests/countries/test_aruba.py (1)🧬 Code Graph Analysis (1)tests/countries/test_aruba.py (2)
🪛 Pylint (3.3.7)tests/countries/test_aruba.py[convention] 24-24: Missing function or method docstring (C0116) [convention] 27-27: Missing function or method docstring (C0116) [convention] 46-46: Missing function or method docstring (C0116) [convention] 49-49: Missing function or method docstring (C0116) [convention] 79-79: Missing function or method docstring (C0116) [convention] 84-84: Missing function or method docstring (C0116) [convention] 97-97: Missing function or method docstring (C0116) [convention] 139-139: Missing function or method docstring (C0116) [convention] 172-172: Missing function or method docstring (C0116) [convention] 192-192: Missing function or method docstring (C0116) [convention] 205-205: Missing function or method docstring (C0116) ⏰ Context from checks skipped due to timeout of 300000ms (5)
🔇 Additional comments (8)
✨ 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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2677 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 251 251
Lines 15286 15272 -14
Branches 2095 2092 -3
=========================================
- Hits 15286 15272 -14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.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 (4)
holidays/locale/nl/LC_MESSAGES/AW.po(3 hunks)holidays/locale/uk/LC_MESSAGES/CW.po(2 hunks)tests/countries/test_aruba.py(7 hunks)tests/countries/test_curacao.py(7 hunks)
🧰 Additional context used
🧠 Learnings (4)
holidays/locale/uk/LC_MESSAGES/CW.po (1)
Learnt from: KJhellico
PR: vacanza/holidays#2651
File: holidays/locale/pap_BQ/LC_MESSAGES/BQ.po:70-72
Timestamp: 2025-06-25T14:08:09.313Z
Learning: The Papiamento translation "Di dos dia di Pasku" for "Tweede Kerstdag" (Boxing Day) in holidays/locale/pap_BQ/LC_MESSAGES/BQ.po comes from an authoritative source, despite the literal meaning potentially seeming to refer to Easter rather than Christmas.
holidays/locale/nl/LC_MESSAGES/AW.po (1)
Learnt from: KJhellico
PR: vacanza/holidays#2651
File: holidays/locale/pap_BQ/LC_MESSAGES/BQ.po:70-72
Timestamp: 2025-06-25T14:08:09.313Z
Learning: The Papiamento translation "Di dos dia di Pasku" for "Tweede Kerstdag" (Boxing Day) in holidays/locale/pap_BQ/LC_MESSAGES/BQ.po comes from an authoritative source, despite the literal meaning potentially seeming to refer to Easter rather than Christmas.
tests/countries/test_curacao.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
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.
tests/countries/test_aruba.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)
tests/countries/test_curacao.py (2)
holidays/countries/curacao.py (2)
Curacao(25-166)CW(169-170)tests/common.py (3)
assertAliases(121-130)assertHolidayName(195-199)assertNoHolidayName(273-275)
tests/countries/test_aruba.py (2)
tests/common.py (6)
setUpClass(32-53)assertAliases(121-130)assertNoHolidays(292-294)assertHolidayName(195-199)assertNoHolidayName(273-275)assertNoHoliday(244-246)holidays/countries/aruba.py (3)
Aruba(24-151)AW(154-155)ABW(158-159)
🪛 Pylint (3.3.7)
tests/countries/test_curacao.py
[convention] 26-26: Missing function or method docstring
(C0116)
[convention] 29-29: Missing function or method docstring
(C0116)
[convention] 33-33: Missing function or method docstring
(C0116)
[convention] 56-56: Missing function or method docstring
(C0116)
[convention] 59-59: Missing function or method docstring
(C0116)
[convention] 71-71: Missing function or method docstring
(C0116)
[convention] 84-84: Missing function or method docstring
(C0116)
[convention] 97-97: Missing function or method docstring
(C0116)
[convention] 166-166: Missing function or method docstring
(C0116)
[convention] 186-186: Missing function or method docstring
(C0116)
[convention] 199-199: Missing function or method docstring
(C0116)
[convention] 212-212: Missing function or method docstring
(C0116)
[convention] 217-217: Missing function or method docstring
(C0116)
[convention] 222-222: Missing function or method docstring
(C0116)
[convention] 230-230: Missing function or method docstring
(C0116)
tests/countries/test_aruba.py
[convention] 24-24: Missing function or method docstring
(C0116)
[convention] 27-27: Missing function or method docstring
(C0116)
[convention] 46-46: Missing function or method docstring
(C0116)
[convention] 49-49: Missing function or method docstring
(C0116)
[convention] 79-79: Missing function or method docstring
(C0116)
[convention] 84-84: Missing function or method docstring
(C0116)
[convention] 97-97: Missing function or method docstring
(C0116)
[convention] 139-139: Missing function or method docstring
(C0116)
[convention] 172-172: Missing function or method docstring
(C0116)
[convention] 192-192: Missing function or method docstring
(C0116)
[convention] 205-205: Missing function or method docstring
(C0116)
🪛 GitHub Actions: CI/CD
tests/countries/test_aruba.py
[error] 245-245: Unit test failure in TestAruba.test_l10n_nl: AssertionError due to mismatch in holiday name capitalization ('Dag van de Arbeid' vs 'Dag van de arbeid'). Please ensure all holiday names are properly localized.
🔇 Additional comments (14)
holidays/locale/uk/LC_MESSAGES/CW.po (2)
17-27: Well-executed localization metadata updates.The version bump to 0.76 and metadata refresh (revision date, translator, Poedit version) maintain proper localization hygiene.
30-92: Excellent standardization and new holiday additions.Good work on:
- Adding trailing periods to all comments for consistency
- Including the new "Whit Sunday" and "Kingdom Day" holidays with proper Ukrainian translations
- The "Kingdom Day" translation aligns with previous feedback
These changes properly support the holiday class refactoring.
tests/countries/test_curacao.py (4)
23-24: Proper test setup alignment with holiday class changes.The start year update to 1955 and addition of half-day holidays instance correctly reflect the refactored Curacao class structure.
30-31: Smart separation of no-holidays assertions.Splitting the test into separate PUBLIC and HALF_DAY category assertions properly validates the conditional holiday logic.
56-237: Comprehensive individual holiday test coverage.Excellent work implementing thorough tests for each holiday:
- Follows the recommended pattern of specific dates followed by range assertions
- Properly tests conditional holidays (Whit Sunday until 2010, Kingdom Day 2008-2009)
- Correctly validates half-day New Year's Eve from 2010 onward
- Year ranges align perfectly with the holiday class implementation
The test structure matches the learned best practices for this codebase.
286-286: Localization test correction applied.Good catch updating the Dutch Christmas Day translation from "Kerst" to "Eerste Kerstdag" for consistency.
holidays/locale/nl/LC_MESSAGES/AW.po (4)
17-17: Version updates look good.Project version and generator updates are appropriate for the refactoring effort.
Also applies to: 27-27
30-30: Comment standardization is consistent.Adding periods to all holiday comments improves consistency across the localization file.
Also applies to: 34-34, 38-38, 42-42, 46-46, 50-50, 54-54, 58-58, 62-62, 74-74, 78-78, 82-82
58-61: Labor Day translation follows previous feedback.The translation "Dag van de Arbeid" aligns with the past review suggestion for proper capitalization.
80-80: Christmas Day translation is more specific.Changing from "Kerst" to "Eerste Kerstdag" better distinguishes the first day of Christmas from the general Christmas term.
tests/countries/test_aruba.py (4)
22-22: Year range aligns with Aruba class start_year.The updated range 1955-2049 matches the start_year in the Aruba holiday class.
46-48: Comprehensive individual holiday tests are well-structured.The new test methods follow the project pattern of testing individual holidays across multiple years and align with the Aruba class implementation.
Also applies to: 51-52, 79-83, 84-96, 97-108, 172-191, 192-204, 205-211
68-77: Holiday transition tests handle year boundaries correctly.The mutual exclusivity tests for carnival/ash monday and queen's/king's day properly validate the holiday logic changes.
Also applies to: 125-126, 137-137, 144-145, 162-162, 169-171
256-256: Christmas Day translation update is consistent.The test correctly reflects the updated Dutch translation "Eerste Kerstdag" from the localization file.
|
|
Hi, it seems this version has a breaking change (at least for the Netherlands). Working code on version 0.75: Running on version 0.76: This will fix this error on version 0.75: |
|
@jovana check out the docs here or here (the Please next time file a new issue instead of commenting on a closed PR. Thanks for using holidays! |
Proposed change
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green