Update Taiwan holidays #2653
Update Taiwan holidays #2653
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe Taiwan holiday logic and data were updated to add and rename holidays, adjust their observance years, and introduce a new pre-Chinese New Year's Eve holiday from 2026. Localization files and tests were revised to match these changes, including translation updates and new/removed holiday entries. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (7)
🧰 Additional context used🧠 Learnings (3)holidays/locale/en_US/LC_MESSAGES/TW.po (1)holidays/countries/taiwan.py (1)tests/countries/test_taiwan.py (1)🪛 Pylint (3.3.7)holidays/countries/taiwan.py[convention] 67-67: Line too long (135/100) (C0301) ⏰ Context from checks skipped due to timeout of 300000ms (2)
🔇 Additional comments (17)
✨ 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 #2653 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 247 247
Lines 15114 15129 +15
Branches 2071 2078 +7
=========================================
+ Hits 15114 15129 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 (8)
holidays/countries/taiwan.py(5 hunks)holidays/groups/chinese.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/TW.po(3 hunks)holidays/locale/th/LC_MESSAGES/TW.po(3 hunks)holidays/locale/zh_CN/LC_MESSAGES/TW.po(3 hunks)holidays/locale/zh_TW/LC_MESSAGES/TW.po(3 hunks)snapshots/countries/TW_COMMON.json(53 hunks)tests/countries/test_taiwan.py(13 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_US/LC_MESSAGES/TW.po (1)
Learnt from: PPsyrius
PR: vacanza/holidays#2437
File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:12-12
Timestamp: 2025-04-06T14:44:59.785Z
Learning: Use standardized .po file headers for localization files in the holidays project:
- Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #"
- Other languages format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #"
tests/countries/test_taiwan.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.
🪛 Pylint (3.3.7)
holidays/groups/chinese.py
[error] 132-132: Instance of 'ChineseCalendarHolidays' has no '_year' member
(E1101)
holidays/countries/taiwan.py
[convention] 67-67: Line too long (135/100)
(C0301)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (25)
holidays/locale/zh_CN/LC_MESSAGES/TW.po (2)
17-29: Localization metadata updates look good.The version bump, date updates, and new source language header are properly maintained.
72-74: New holiday translation added appropriately.The Taiwan Restoration and Guningtou Victory Memorial Day has been properly localized to Simplified Chinese.
holidays/groups/chinese.py (1)
124-133: New Chinese calendar method implementation looks solid.The method correctly calculates the day before Chinese New Year's Eve using
days_delta=-2and follows the established pattern of other methods in the class. The static analysis hint about_yearis a false positive since this class inherits fromEasternCalendarHolidayswhich provides that member.holidays/locale/zh_TW/LC_MESSAGES/TW.po (2)
17-29: Metadata updates are consistent.The version and date updates match the other localization files.
72-74: Empty translation string needs verification.The new holiday entry has an empty translation string. Since this is the source language (zh_TW), verify if this is intentional or if the translation should be populated.
holidays/locale/th/LC_MESSAGES/TW.po (2)
17-29: Localization metadata properly updated.Consistent with the other language files in this update.
72-74: Thai translation added appropriately.The holiday has been properly localized with a comprehensive Thai translation.
holidays/locale/en_US/LC_MESSAGES/TW.po (2)
17-29: Metadata updates are consistent and proper.The version bump and date updates align with the other localization files.
72-74: English translation is clear and appropriate.The holiday name translation accurately reflects the original meaning and provides good context.
snapshots/countries/TW_COMMON.json (3)
19-19: LGTM! Consistent Teacher's Day removal from Confucius' Birthday.The systematic removal of "Teacher's Day" from "Confucius' Birthday; Teacher's Day" entries reflects the regulatory changes properly. Data consistency looks solid across all years.
Also applies to: 43-43, 68-68, 90-90, 113-113, 137-137, 158-158, 181-181, 205-205, 236-236, 259-259, 285-285, 313-313, 338-338, 363-363, 393-393, 419-419, 449-449, 479-479, 507-507, 536-536, 563-563, 592-592, 625-625, 653-653, 684-684, 713-713, 738-738, 768-768, 798-798, 825-825, 854-854, 881-881, 907-907, 937-937, 966-966, 994-994, 1022-1022, 1048-1048, 1079-1079, 1108-1108, 1136-1136, 1166-1166, 1193-1193, 1219-1219, 1250-1250, 1278-1278, 1307-1307, 1334-1334, 1360-1360, 1389-1389, 1419-1419, 1447-1447
742-742: Holiday name change properly implemented from 2025 onwards."Taiwan Retrocession Day" correctly transitions to "Taiwan Restoration and Guningtou Victory Memorial Day" starting 2025, maintaining historical accuracy for earlier years.
Also applies to: 772-772, 802-802, 829-829, 857-857, 884-884, 911-911, 941-941, 969-969, 997-997, 1025-1025, 1053-1053, 1083-1083, 1112-1112, 1141-1141, 1169-1169, 1196-1196, 1224-1224, 1254-1254, 1282-1282, 1310-1310, 1337-1337, 1364-1364, 1393-1393, 1423-1423, 1451-1451
746-746: New Chinese New Year's Eve entries align with 2026 implementation.The additional Chinese New Year's Eve dates starting from 2026 properly reflect the new "Chinese day before New Year's Eve" holiday addition.
Also applies to: 751-751, 776-776, 807-807, 833-833, 838-838, 861-861, 888-888, 915-915, 946-946, 952-952, 974-974, 1001-1001, 1029-1029, 1035-1035, 1057-1057, 1087-1087, 1117-1117, 1123-1123, 1146-1146, 1173-1173, 1200-1200, 1228-1228, 1233-1233, 1258-1258, 1287-1287, 1314-1314, 1319-1319, 1341-1341, 1368-1368, 1397-1397, 1402-1402, 1428-1428
holidays/countries/taiwan.py (5)
127-130: Chinese New Year's Eve enhancement looks solid.The addition of "Chinese day before New Year's Eve" from 2026 while maintaining the existing holiday is well-implemented. Logic correctly adds both holidays when appropriate.
148-151: Labor Day transition to public holiday properly implemented.Moving Labor Day from optional to public holidays starting 2026 aligns with the regulatory changes and maintains backward compatibility.
158-161: New public holidays correctly added from 2025.Confucius' Birthday, Taiwan Restoration and Guningtou Victory Memorial Day, and Constitution Day properly elevated to public holiday status with appropriate year conditions.
Also applies to: 165-171
206-208: Labor Day removal from optional holidays properly scoped.Limiting Labor Day in optional holidays to 2025 and earlier maintains consistency with its elevation to public holiday status from 2026.
248-261: Workday holiday transitions handled correctly.The year-based conditions properly limit Confucius' Birthday, Taiwan Retrocession Day, and Constitution Day to workday holidays through 2024, supporting their transition to public holidays from 2025.
tests/countries/test_taiwan.py (8)
186-189: LGTM on Chinese New Year's Eve test updates.The addition of 2025-2026 test dates correctly implements the new Chinese day before New Year's Eve holiday logic. The test structure follows existing patterns and the dates align with the expected changes.
Also applies to: 243-243
467-475: Well-structured Labor Day category transition.The transition from optional to public holiday starting 2026 is correctly implemented with proper year range splits and category exclusions.
600-602: Correct implementation of Confucius' Birthday upgrade.The transition from workday to public holiday starting 2025 is properly structured with accurate year range handling.
Also applies to: 618-620
633-659: Solid handling of Taiwan Retrocession Day rename and upgrade.The implementation cleanly separates old vs new holiday names and correctly transitions from workday to public holiday starting 2025.
717-719: Clean Constitution Day category transition.The upgrade from workday to public holiday starting 2025 follows the established pattern with correct year range exclusions.
Also applies to: 735-737
1265-1265: Comprehensive 2025 test case updates.The addition of all three new public holidays (Confucius' Birthday, renamed Taiwan Retrocession Day, Constitution Day) correctly validates the 2025 changes.
Also applies to: 1268-1269
1294-1294: Consistent localization updates across all variants.The removal of Teacher's Day references from September 28 is properly implemented across all supported locales (default, en_US, th, zh_CN).
Also applies to: 1325-1325, 1356-1356, 1387-1387
467-475: Excellent overall implementation of 2025 Taiwan holiday updates.All holiday transitions are consistently implemented with proper year range handling and category changes. The test structure adheres to best practices by organizing tests by individual holidays rather than years.
Also applies to: 600-602, 617-620, 633-659, 717-719, 735-737
|
I removed Teachers' Day because it is quite obviously identical with Confucius' Birthday, and changed Retrocession Day name (but since 2025 only). @PPsyrius, You've researched Taiwan in more detail, and are more familiar with the sources, so please take a look - maybe I'm misinterpreting something. And of course, check out Thai translation of Retrocession Day new name. :) |
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: ~Jhellico <KJhellico@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
holidays/locale/th/LC_MESSAGES/TW.po (1)
72-75: Approve translation for new holiday entryThe Thai msgstr for
"臺灣光復暨金門古寧頭大捷紀念日"matches PPsyrius’s prior suggestion and accurately conveys the intended meaning. LGTM.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/locale/th/LC_MESSAGES/TW.po(3 hunks)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Run pre-commit
|
Proposed change
Update Taiwan holidays: add new public holidays according to 2025 regulatory changes.
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green