-
-
Notifications
You must be signed in to change notification settings - Fork 561
Update Netherlands holidays: add th l10n, refactor test cases
#3006
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
Conversation
Summary by CodeRabbit
WalkthroughNetherlands holiday rules were revised (start year, Good Friday, King/Queen’s Day movement, Liberation Day cycle), Thai localization for NL was added, README language list updated, NL snapshot data refreshed, and Netherlands tests were expanded and refactored. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3006 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 306 306
Lines 18076 18069 -7
Branches 2334 2331 -3
=========================================
- Hits 18076 18069 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)holidays/countries/netherlands.py(3 hunks)holidays/locale/en_US/LC_MESSAGES/NL.po(2 hunks)holidays/locale/fy/LC_MESSAGES/NL.po(2 hunks)holidays/locale/nl/LC_MESSAGES/NL.po(2 hunks)holidays/locale/th/LC_MESSAGES/NL.po(1 hunks)holidays/locale/uk/LC_MESSAGES/NL.po(2 hunks)snapshots/countries/NL_COMMON.json(7 hunks)tests/countries/test_netherlands.py(2 hunks)
🧰 Additional context used
🧠 Learnings (19)
📚 Learning: 2025-05-06T15:25:44.333Z
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: holidays/locale/ca/LC_MESSAGES/AD.po:31-40
Timestamp: 2025-05-06T15:25:44.333Z
Learning: In the Holidays project, msgid fields in localization files contain strings in the entity's default language (as defined by default_language attribute), not English source strings as in standard gettext implementations.
Applied to files:
holidays/locale/en_US/LC_MESSAGES/NL.poholidays/locale/fy/LC_MESSAGES/NL.poholidays/locale/uk/LC_MESSAGES/NL.poholidays/locale/nl/LC_MESSAGES/NL.po
📚 Learning: 2025-03-05T17:51:00.633Z
Learnt from: KJhellico
PR: vacanza/holidays#2259
File: holidays/locale/en_IN/LC_MESSAGES/IN.po:30-299
Timestamp: 2025-03-05T17:51:00.633Z
Learning: In the Holidays project, .po files for a country's default locale use empty msgstr fields as a standard convention.
Applied to files:
holidays/locale/en_US/LC_MESSAGES/NL.poholidays/locale/fy/LC_MESSAGES/NL.poholidays/locale/uk/LC_MESSAGES/NL.poholidays/locale/nl/LC_MESSAGES/NL.po
📚 Learning: 2025-06-28T10:39:19.185Z
Learnt from: KJhellico
PR: vacanza/holidays#2684
File: holidays/locale/it/LC_MESSAGES/SM.po:13-13
Timestamp: 2025-06-28T10:39:19.185Z
Learning: In the holidays project, .po file header comments use the format "# [Country] holidays." for default language files (without trailing hash) and "# [Country] holidays [locale] localization." for non-default language files (also without trailing hash).
Applied to files:
holidays/locale/en_US/LC_MESSAGES/NL.poholidays/locale/fy/LC_MESSAGES/NL.poholidays/locale/uk/LC_MESSAGES/NL.poholidays/locale/nl/LC_MESSAGES/NL.po
📚 Learning: 2025-06-25T10:09:29.029Z
Learnt from: PPsyrius
PR: vacanza/holidays#2676
File: holidays/locale/ar/LC_MESSAGES/EG.po:0-0
Timestamp: 2025-06-25T10:09:29.029Z
Learning: In the holidays library, msgstr fields can be left empty for source/default_language files when using Lingva, the localization tool used by the project.
Applied to files:
holidays/locale/en_US/LC_MESSAGES/NL.poholidays/locale/fy/LC_MESSAGES/NL.poholidays/locale/uk/LC_MESSAGES/NL.poholidays/locale/nl/LC_MESSAGES/NL.po
📚 Learning: 2025-09-26T13:58:49.363Z
Learnt from: KJhellico
PR: vacanza/holidays#2960
File: holidays/locale/ca/LC_MESSAGES/ES.po:148-151
Timestamp: 2025-09-26T13:58:49.363Z
Learning: In the holidays project, when translating holiday names in .po files, the msgstr should be a faithful translation of the original msgid (in the country's default language), not influenced by English commentary or common en_US names. For example, "Día de la Pascua Granada" (Spanish) correctly translates to "Dia de la Pasqua Granada" (Catalan), even though the English comment refers to it as "Whit Monday".
Applied to files:
holidays/locale/en_US/LC_MESSAGES/NL.poholidays/locale/fy/LC_MESSAGES/NL.poholidays/locale/uk/LC_MESSAGES/NL.poholidays/locale/nl/LC_MESSAGES/NL.po
📚 Learning: 2025-06-25T14:08:09.323Z
Learnt from: KJhellico
PR: vacanza/holidays#2651
File: holidays/locale/pap_BQ/LC_MESSAGES/BQ.po:70-72
Timestamp: 2025-06-25T14:08:09.323Z
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.
Applied to files:
holidays/locale/en_US/LC_MESSAGES/NL.poholidays/locale/fy/LC_MESSAGES/NL.poholidays/locale/uk/LC_MESSAGES/NL.poholidays/locale/nl/LC_MESSAGES/NL.po
📚 Learning: 2025-09-20T12:21:50.877Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_belgium.py:28-30
Timestamp: 2025-09-20T12:21:50.877Z
Learning: Belgium holidays implementation currently lacks a start_year attribute. In tests/countries/test_belgium.py, do not suggest adding test_no_holidays methods that rely on start_year until the start_year attribute is introduced to Belgium's holiday implementation.
Applied to files:
holidays/countries/netherlands.pytests/countries/test_netherlands.py
📚 Learning: 2025-04-05T04:47:27.213Z
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.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-05-06T21:07:11.577Z
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-04-05T06:49:06.217Z
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-09-18T03:19:23.722Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_algeria.py:28-30
Timestamp: 2025-09-18T03:19:23.722Z
Learning: In the vacanza/holidays project, tests now use self.start_year and self.end_year from the TestCase class instead of country-specific aliases (like DZ.start_year) for start_year and end_year references. This approach provides the test framework with better control over test year ranges rather than being tied to specific country start years.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-07-24T15:21:31.632Z
Learnt from: PPsyrius
PR: vacanza/holidays#2750
File: tests/countries/test_germany.py:46-46
Timestamp: 2025-07-24T15:21:31.632Z
Learning: In the holidays project test files, the standard method name for testing the absence of holidays is `test_no_holidays`, not more descriptive names like `test_no_holidays_before_1990`. This is a consistent naming convention across country test files like France and Germany.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-09-28T05:42:12.777Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_indonesia.py:221-223
Timestamp: 2025-09-28T05:42:12.777Z
Learning: In tests/countries/test_indonesia.py, the manual set inclusion checks using get_named and years_found for Lunar New Year (test_lunar_new_year) and Vesak Day (test_vesak_day) are intentional and should remain until both holidays get their own `{insert}_no_estimated` flags implemented, rather than using standard harness assertions like assertHolidayName/assertNoHolidayName.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-05-09T18:36:09.607Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: tests/countries/test_finland.py:23-26
Timestamp: 2025-05-09T18:36:09.607Z
Learning: The holidays project prioritizes complete historical coverage in tests, verifying holidays from their first year of observance (e.g., 1853 for Finland) through future projections, rather than using shorter sliding windows.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-09-14T04:41:10.139Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_south_africa.py:22-22
Timestamp: 2025-09-14T04:41:10.139Z
Learning: South Africa's observed holiday system only started in 1995, so in tests/countries/test_south_africa.py, using years_non_observed=range(1995, 2050) is intentional to limit testing to years where observed holidays actually exist, improving both correctness and performance.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-09-10T14:35:54.603Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_brazil.py:28-30
Timestamp: 2025-09-10T14:35:54.603Z
Learning: In the holidays project, the test_no_holidays method should test ALL supported_categories (via categories=CountryClass.supported_categories) rather than just the default PUBLIC category, to ensure comprehensive validation that no holidays exist before start_year across any supported category including OPTIONAL and subdivision categories.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-09-20T12:24:28.864Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_curacao.py:29-29
Timestamp: 2025-09-20T12:24:28.864Z
Learning: For Curacao in the holidays library, PUBLIC and HALF_DAY categories have different start years - PUBLIC holidays begin at the country's start_year while HALF_DAY holidays begin in 2010. When testing no_holidays methods, each category should be tested separately with appropriate pre-start years rather than using a unified supported_categories approach.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-09-14T16:03:13.558Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_barbados.py:21-23
Timestamp: 2025-09-14T16:03:13.558Z
Learning: In tests/countries/test_barbados.py, using years_non_observed=range(2000, 2024) is intentional because all observed holiday examples fall within 2001-2023, making this range appropriate for limiting testing to years where observed holidays actually exist in the test data.
Applied to files:
tests/countries/test_netherlands.py
📚 Learning: 2025-06-25T20:55:00.642Z
Learnt from: KJhellico
PR: vacanza/holidays#2651
File: holidays/locale/nl/LC_MESSAGES/BQ.po:29-95
Timestamp: 2025-06-25T20:55:00.642Z
Learning: In the holidays library, Dutch locale files (.po) with `X-Source-Language: nl` should have empty msgstr entries when the target language is also Dutch. The library uses fallback=True with gettext, which returns the original msgid when msgstr is empty. This is the correct pattern for native language files and does not cause blank holiday names.
Applied to files:
holidays/locale/th/LC_MESSAGES/NL.po
🧬 Code graph analysis (2)
holidays/countries/netherlands.py (3)
holidays/groups/international.py (1)
_add_new_years_day(142-150)holidays/groups/christian.py (3)
_add_good_friday(344-353)_add_easter_sunday(286-295)_add_easter_monday(275-284)holidays/observed_holiday_base.py (1)
_move_holiday(199-211)
tests/countries/test_netherlands.py (3)
tests/countries/test_slovakia.py (1)
setUpClass(22-25)tests/common.py (6)
setUpClass(35-56)assertAliases(124-133)assertNoHolidays(324-326)assertHolidayName(198-202)assertNoHolidayName(305-307)assertLocalizedHolidays(359-370)holidays/countries/netherlands.py (3)
Netherlands(20-98)NL(101-102)NLD(105-106)
🔇 Additional comments (15)
holidays/locale/nl/LC_MESSAGES/NL.po (2)
17-17: Header bump OK.Version to 0.83 looks good; nl default-locale keeps empty msgstr entries.
34-37: Good Friday entry correctly added and positioned.Placement before Easter Sunday matches convention; empty msgstr is correct for nl default locale.
holidays/locale/fy/LC_MESSAGES/NL.po (2)
17-17: Header bump OK.Project-Id-Version updated to 0.83; metadata consistent.
33-36: Good Friday translation LGTM."Goede Vrijdag" → "Goedfreed" is accurate; ordering is consistent with other locales.
holidays/locale/uk/LC_MESSAGES/NL.po (2)
17-17: Header bump OK.Metadata update to 0.83 is fine.
35-37: Good Friday translation LGTM."Страсна пʼятниця" is correct; order consistent with other locales.
holidays/countries/netherlands.py (3)
39-41: Start year and language set are appropriate.Atw alignment with start_year=1965 and adding "th" to supported_languages look good.
50-57: New Year gating and Good Friday inclusion make sense.
- New Year only from 1966 (since 1965-01-01 precedes Atw) is correct.
- Good Friday added unconditionally within start_year scope is fine.
64-71: Weekend move rule for King’s/Queen’s Day looks correct.SUN→prev Sat from 1980; otherwise SUN→next Mon matches historical handling.
holidays/locale/th/LC_MESSAGES/NL.po (1)
54-57: Fix Thai translation for Liberation Day."วันประกาศอิสรภาพ" means “Independence Day”; replace with the standard Thai term “วันปลดปล่อย”.
Apply this diff:
msgid "Bevrijdingsdag" -msgstr "วันประกาศอิสรภาพ" +msgstr "วันปลดปล่อย"README.md (1)
1225-1225: NL supported languages update looks correct (added th).
Consistent with Netherlands.supported_languages.holidays/locale/en_US/LC_MESSAGES/NL.po (1)
17-27: Header bump and Good Friday translation placement look good.
- en_US msgstr entries correctly translate nl msgids.
- Good Friday entry is present once and positioned logically before Easter.
Based on learnings
Also applies to: 34-37
snapshots/countries/NL_COMMON.json (1)
2-12: Snapshot aligns with new NL rules (start_year=1965, Bevrijdingsdag cadence, merged names when overlapping).
Looks consistent with tests and implementation.If your snapshot generator intentionally includes OPTIONAL along with PUBLIC in COMMON snapshots, no action needed. Otherwise, confirm Bevrijdingsdag presence matches intended category inclusion policy for snapshots.
Also applies to: 180-206, 428-436, 552-556
tests/countries/test_netherlands.py (2)
23-26: Class setup is solid.
Year range and OPTIONAL instance construction follow project patterns.
30-32: Good coverage for “no holidays” pre-start_year across all categories.
Matches project guidance to validate supported_categories, not just PUBLIC.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com>
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.
LGTM!
|
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.
LGTM 👍
Proposed change
th) l10n for the Netherlands holidays.start_yearto 1966 instead (the year in which the Algemene termijnenwet (Atw) became effective for full year).Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.