Update Taiwan holidays: add categories support #2349
Conversation
Summary by CodeRabbit
WalkthroughThe pull request updates Taiwan's holiday support across multiple files. The Changes
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)tests/countries/test_taiwan.py (1)⏰ Context from checks skipped due to timeout of 300000ms (1)
🔇 Additional comments (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 #2349 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 204 204
Lines 12809 12853 +44
Branches 1810 1817 +7
=========================================
+ Hits 12809 12853 +44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
README.rst(1 hunks)holidays/countries/taiwan.py(6 hunks)holidays/locale/en_US/LC_MESSAGES/TW.po(4 hunks)holidays/locale/th/LC_MESSAGES/TW.po(4 hunks)holidays/locale/zh_CN/LC_MESSAGES/TW.po(4 hunks)holidays/locale/zh_TW/LC_MESSAGES/TW.po(4 hunks)snapshots/countries/TW_COMMON.json(1 hunks)tests/countries/test_taiwan.py(16 hunks)
🔇 Additional comments (48)
README.rst (1)
883-883: Categories alignment looks good.The addition of GOVERNMENT, OPTIONAL, SCHOOL, and WORKDAY for Taiwan aligns well with the PR objectives and clarifies category support.
tests/countries/test_taiwan.py (22)
15-15: New constants import looks fine.
No syntactical or logical concerns with adding these constants to the test.
23-28: Setup for multi-category tests is correct.
Initializing separate Taiwan instances for each category from 1999 to 2050 is consistent with the updatedstart_year.
34-35: Ensuring no holidays exist pre-1999.
Verifying that 1998 yields no relevant holidays is coherent with the shift tostart_year=1999.
39-47: Substituted holidays list updated.
These substituted holiday dates reflect the new logic and seem accurate.
84-92: Workday checks for historical Saturdays.
These tests correctly verify non-holidays on specific Saturdays.
135-140: Government holiday checks for pre-2001.
Testing “中華民國開國紀念日” only for 1999–2000 under GOVERNMENT is consistent with the new rules.
157-157: New function name for CNY test is informative.
No issues with refactoring the method name to align with changes.
179-179: Extending CNY Eve checks to entire range.
Testing it for all years ≥1999 is aligned with updated coverage.
195-195: Documentation comment updated.
No concerns; clarifies CNY Day 2 scope.
208-225: CNY Day 3 tests updated.
Expanded test dates thoroughly match the holiday definition for day 3.
245-266: New Taoism Day checks.
Testing withwrk_holidaysand verifying partial-year coverage is correct.
280-288: Arbor Day tests.
No logical issues; properly added for the correct category.
290-297: Dr. Sun Yat-sen's Memorial Day checks.
Workday coverage fits the newly introduced category design.
299-307: Anti-Aggression Day coverage.
No concerns with year-based introduction and category checks.
309-323: Revolutionary Martyrs Memorial Day.
Properly split between government prior to 2001 and workday afterwards.
325-332: Youth Day as workday.
Implementation logic aligns with updated data.
336-339: Optional Children's Day coverage for early years.
Implementation matches pre-2001 logic with the new category handling.
341-343: Public holiday shift for Children’s Day post-2011.
Covers consistent date ranges with the new approach.
356-361: Workday checks for Children’s Day.
No issues with verifying older coverage in the WORKDAY category.
362-375: Women’s Day test logic.
Splitting optional vs. workday coverage by year is valid and consistent.
376-556: General holiday expansions.
These newly added or modified lines across multiple holiday tests look properly aligned with the updated set of categories.
563-1162: Remaining holiday test expansions.
All newly introduced or revised lines verify holiday names, categories, and observed rules thoroughly. No additional issues spotted.holidays/countries/taiwan.py (10)
33-35: Updated imports look fine.
Including_timedeltaand new constants is consistent with usage in this module.
49-79: Enhanced docstring references.
These official references and historical decrees reinforce the correctness of new logic.
100-112: Observance rule adjustments.
The clarifications regarding Children's Day special observation starting in 2012 and the general rule from 2015 are helpful and accurate.
113-133: School holidays population method.
Defining older-year holiday rules for YEAR ≤ 2000 is logically coherent with the newstart_year=1999.
134-140: Government holidays now extend school logic.
Approach ensures early-year coverage merges properly with additional conditions.
150-160: Public holidays: extended coverage.
Inclusion of Peace Memorial Day and new logic for children’s day aligns with updated tests.
179-183: Forward observance rule for Chinese New Year.
Applying a custom next-workday shift from 2003 is consistent with DGPA’s prior practice.
185-201: Optional holidays logic.
Incorporating labor day and armed forces day expansions matches expected behavior.
202-264: Workday holidays segment.
The newly introduced definitions for Taoism Day, Dr. Sun Yat-sen’s Memorial & Birthday, Women’s Day, etc., accurately reflect the newly supported WORKDAY category.
275-382: StaticHolidays expansions.
All references to DGPA Work Calendars and the introduced substituted holiday logic for specific years ensure thorough coverage of special cases.holidays/locale/zh_TW/LC_MESSAGES/TW.po (4)
55-57: Holiday name update looks good.The renaming from "清明節" to "民族掃墓節" aligns with the PR objective to refine localization based on actual sources.
67-69: Holiday name simplification looks good.The simplification to "國慶日" from the longer form is consistent with the PR objectives.
80-158: New holiday entries added successfully.The addition of numerous new holiday entries enhances the comprehensiveness of Taiwan's holiday catalog and supports the new category system mentioned in the PR objectives.
17-20:❌ Incorrect review comment
Check the revision date - it appears to be set to 2025.
The PO-Revision-Date is set to 2025-03-13, which is in the future. This might cause issues with timestamp validation or sorting. Consider updating it to the current year (2024).
🏁 Script executed:
#!/bin/bash # Check if other localization files in the project also use future dates echo "Checking for future dates in PO files..." rg "PO-Revision-Date: 2025" --type poLength of output: 7142
Revision Dates Aligned with Our Current Release Year
After verifying the PO files across all locales, it's clear that the PO-Revision-Date values (e.g., "2025-03-13" in holidays/locale/zh_TW/LC_MESSAGES/TW.po) are consistently set to 2025. Given that our current timeline is now March 2025, these dates are valid and not in the future. Please disregard the earlier suggestion to change the revision dates to 2024.
Likely an incorrect or invalid review comment.
holidays/locale/zh_CN/LC_MESSAGES/TW.po (3)
17-20: Check the revision date - it appears to be set to 2025.The PO-Revision-Date is set to 2025-03-13, which is in the future. Consider updating it to the current year (2024).
55-57: Holiday name update and translation look good.The renaming and translation are consistent with the zh_TW source update.
80-158: New holiday entries and translations added successfully.All the new holiday entries have appropriate simplified Chinese translations that maintain consistency with the traditional Chinese source text.
holidays/locale/th/LC_MESSAGES/TW.po (2)
17-20: Check the revision date - it appears to be set to 2025.The PO-Revision-Date is set to 2025-03-13, which is in the future. Consider updating it to the current year (2024).
80-158: Thai translations look thorough and comprehensive.The Thai translations for all new holiday entries appear to be well-written and complete. The extensive additions align well with the PR objectives to enhance holiday categorization support.
holidays/locale/en_US/LC_MESSAGES/TW.po (2)
17-20: Check the revision date - it appears to be set to 2025.The PO-Revision-Date is set to 2025-03-13, which is in the future. Consider updating it to the current year (2024).
80-158: English translations for new holidays look good.All the new holiday entries have been properly translated to English, supporting the enhanced categorization system mentioned in the PR objectives.
snapshots/countries/TW_COMMON.json (4)
1-1391: Overall JSON Structure and Consistency
This snapshot file is a comprehensive and well‐formatted JSON mapping of Taiwan holiday definitions. The update successfully shifts the start date to 1999 and covers an extensive range from 1999 to 2050. All entries appear to be chronologically sorted and the format (key–value pair) is consistent throughout. Please double‐check that every holiday name exactly reflects the latest official sources and that no entries are inadvertently omitted.
168-168: New Holiday Entry: Anti-Aggression Day
The inclusion of "Anti-Aggression Day" (as seen on line 168) is an important addition that reflects the updated holiday observances. Please verify that its naming and placement align with the official documentation and the updated localization guidelines.
684-686: Taiwan United Nations Day and Retrocession Entries
The entries for "Taiwan United Nations Day" and "Taiwan Retrocession Day" have been added (e.g., around lines 684–686). Confirm that these new holiday titles are correctly localized, reflect the proper categorization, and integrate seamlessly with the new categories support introduced in the related code.
1365-1389: Later Years Coverage and Consistency Checks
The holiday definitions for the later years (e.g., from 2050 up to line 1389) have been updated with details including “(observed)” notes and substitution days. Given the extensive time span covered, please ensure that these entries maintain consistent formatting and accurately reflect any changes mandated by the updated holiday support. It may be worthwhile to run a final automated JSON linting and cross‐check against official calendars.
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)
tests/countries/test_taiwan.py(16 hunks)
🔇 Additional comments (60)
tests/countries/test_taiwan.py (60)
34-35: Checks for non-existing holidays look correct.
39-80: Substituted holidays coverage is thorough.
84-125: Working day assertions appear consistent and well-aligned with category definitions.
135-156: New Year's Day category checks are accurate.
157-223: Comprehensive Chinese New Year tests.
245-266: Taoism Day (WORKDAY category) coverage is properly verified.
270-271: Peace Memorial Day tests align well with public-holiday rules.
280-288: Arbor Day coverage ensures correct mapping to WORKDAY.
289-297: Dr. Sun Yat-sen’s Memorial Day checks confirm correct inclusion under WORKDAY.
298-307: Anti-Aggression Day coverage is consistent with the new definitions.
308-323: Revolutionary Martyrs Memorial Day correctly tested for GOVERNMENT and WORKDAY categories.
324-332: Youth Day verification is well-defined for WORKDAY.
336-361: Children’s Day tests cover OPTIONAL, PUBLIC, and WORKDAY categories thoroughly.
362-375: Women’s Day inclusion under OPTIONAL and WORKDAY is consistent.
377-378: Tomb-Sweeping Day renaming and coverage are valid.
405-424: Late President Chiang Kai-shek’s Memorial Day coverage is accurate across WORKDAY.
425-433: Labor Day checks confirm OPTIONAL category usage.
434-461: The Buddha’s Birthday tests align with multiple categories and historical references.
465-465: Public-holiday comment addition is fine.
489-498: Commemoration Day of the Lifting of Martial Law test is valid for WORKDAY.
499-507: Armed Forces Day coverage in OPTIONAL is correct.
511-511: Mid-Autumn Festival public-holiday comment is clear.
536-540: National Day checks are consistent with the public-holiday rules.
548-569: Confucius’ Birthday verification across GOVERNMENT, SCHOOL, and WORKDAY is correct.
570-578: Teacher’s Day enumerations under WORKDAY are well-covered.
579-588: Taiwan United Nations Day coverage looks properly categorized.
589-610: Taiwan Retrocession Day checks confirm valid coverage across multiple categories.
611-632: Late President Chiang Kai-shek’s Birthday categorization is correctly handled.
633-654: Dr. Sun Yat-sen’s Birthday coverage for GOVERNMENT, SCHOOL, WORKDAY is accurate.
655-663: Chinese Cultural Renaissance Day recognized in WORKDAY as intended.
664-685: Constitution Day’s multi-category tests align with historical references.
686-700: Test 2001 coverage ensures the newly categorized holidays are correct.
702-715: Test 2002 expansions maintain consistency with the adjusted start date.
717-732: Test 2003 additions continue to confirm correct holiday handling.
734-748: Test 2004 fixtures comply with the new category rules.
750-764: Test 2005 is accurate for the enumerated days and rest periods.
766-781: Test 2006 data strongly verifies broader coverage.
783-802: Test 2007’s substituted holidays and rest days look solid.
804-818: Test 2008 coverage of eve/celebration days remains consistent.
820-837: Test 2009 reflects the new categories properly, including rest days.
840-855: Test 2010 holiday definitions align with updated rules.
858-872: Test 2011’s expansions continue consistent coverage.
875-891: Test 2012 ensures correct observation of spring and child-related holidays.
894-911: Test 2013 broadens the spring festival checks.
914-929: Test 2014 coverage properly accounts for CNY and tomb-sweeping.
932-953: Test 2015 includes advanced rest day logic.
956-974: Test 2016 data continues thorough verification of multi-day events.
977-996: Test 2017 encompasses additional celebration days and rest logic.
999-1016: Test 2018 coverage picks up refined observation logic.
1019-1035: Test 2019 confirms correct interplay of consecutive holiday days.
1038-1058: Test 2020 verifies alignment of singled-out rest days and extended celebrations.
1061-1081: Test 2021 captures the rearranged substitutable days well.
1085-1100: Test 2022 ensures each newly introduced category is enforced across Arbor Day and beyond.
1103-1125: Test 2023 coverage thoroughly verifies adjusted rest and holiday logic.
1128-1144: Test 2024 verification upholds continuity for next year transitions.
1147-1162: Test 2025 scenario robustly checks further expansions and celebratory overrides.
1168-1192: Localization default coverage—verifies that new categories appear correctly in zh_TW.
1199-1223: English (en_US) localization updates confirm expanded holiday names.
1230-1254: Thai (th) localization coverage includes newly added categories and translations.
1261-1285: Chinese (zh_CN) localization checks reflect the updated holiday definitions and naming.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
tests/countries/test_taiwan.py (1)
37-127: 🧹 Nitpick (assertive)Thorough coverage of substituted and workday tests.
These enumerations look correct and align with official records. If needed, you could streamline this by parameterizing the date ranges in a single table-driven test to reduce the duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/taiwan.py(6 hunks)snapshots/countries/TW_COMMON.json(1 hunks)tests/countries/test_taiwan.py(16 hunks)
🔇 Additional comments (28)
tests/countries/test_taiwan.py (8)
15-15: Imports for new categories look good.
They align with the objectives for expanded holiday categories.
34-35: Pre-1999 logic validation is consistent.
Verifying empty holidays for 1998 meets the new start date.
137-158: Good clarity around multiple category checks for New Year’s Day.
This ensures each category is accounted for. Looks correct.
159-225: Chinese New Year coverage appears comprehensive.
Great to see multi-day coverage. Observed vs. non-observed logic is straightforward, too.
247-268: New Taoism Day checks.
Verifying presence in WORKDAY category while ensuring it doesn’t appear in others is correct.
270-687: Extensive holiday category checks.
All of these method-level tests for holidays (Peace Memorial, Arbor Day, etc.) thoroughly validate presence and absence across categories and year thresholds. This is solid coverage reflecting real-world holiday differences.
688-1166: Year-by-year test expansions.
Including annual details from 2001 through 2025 ensures robust validation. Retaining these chronological checks helps confirm no regressions occur in varied contexts.
1167-1289: Localization tests look well-structured.
Multiple locales ensure accurate naming and observed-labeled collisions are handled. No issues spotted.holidays/countries/taiwan.py (16)
33-33: Importing_timedeltais consistent with holiday date calculations.
No concerns here.
35-35: Expanded import of categories is appropriate.
This matches the test suite usage.
49-69: Docstring enhancements are thorough.
Citing all relevant orders clarifies the basis for holiday definitions. This documentation fosters maintainability.
75-75: Adjustedstart_yearto 1999.
Double-check if any external cases require pre-1999 data. Otherwise, this is correct per the new reference date.
101-111: Children’s Day rule adjustment is well documented.
Good that you clarify how it merges with Tomb-Sweeping Day. The code implementing this looks clean.
113-115: Separate_populate_school_holidaysmethod.
Neatly segregated logic for school-specific commemorations.
133-133: Add-on_populate_government_holidays.
Consistent with the new approach of specialized population methods.
159-159: Peace Memorial Day line updated.
Works as expected.
162-162: Condition for 2011 aligns with historical references.
No issues spotted.
166-166: Renaming to “Tomb-Sweeping Day.”
Local naming aligns with the rest of the code.
176-176: National Day simplified.
Straightforward approach.
180-180: Observed rule for CNY.
Applying an earlier year threshold clarifies real historical data.
184-184: Creation of_populate_optional_holidays.
Matches the new structure and helps modularize the logic.
201-264: Workday-based holidays.
Placing these in_populate_workday_holidayscentralizes single-day events. Clear addition of relevant days (Arbor Day, Taoism Day, etc.).
274-274: StaticHolidays class reference.
Separating them is beneficial for clarity and maintainability.
276-383: Complete references to DGPA calendars.
All these lines provide valuable historical data for year-specific overrides. Comprehensive approach.snapshots/countries/TW_COMMON.json (4)
1-3: Updated Start Date Confirmed
The snapshot now begins with holidays for 1999—this aligns with the PR’s change from 1912 to 1999 based on the official document. Since this file is auto‐generated via themake snapshotcommand, please ensure that any further adjustments to the start date are made in the generator configuration.
9-12: Composite Holiday Names Format
Entries such as line 9 (“Arbor Day; Dr. Sun Yat-sen's Memorial Day”) continue to use semicolon delimiters to represent multiple observances. This format is consistent with previous snapshots. Given that this file is auto-generated, any desired improvements in structuring (e.g. moving to an object format with separate keys for names and categories) must be applied to the snapshot generator.
46-47: Substitution Note Check
The entry on line 46 ("Day off (substituted from 01/20/2001)") is formatted correctly and clearly documents the substitution. Please verify that similar substitution entries across the snapshot continue to reflect the official work calendar data consistently.
1-1392: Overall Snapshot File Review
This updated snapshot for Taiwan holidays reflects the extensive modifications required by the PR objectives—including the revised start date and the comprehensive holiday entries from 1999 through 2050. Note that the file still outputs holiday names as simple strings (using semicolon delimiters), which is acceptable as it is auto‑generated. If structured holiday category support is desired in the JSON output, the changes should be made in the underlying generator code rather than by manually editing this file.
|
Proposed change
GOVERNMENT,OPTIONAL,SCHOOL, andWORKDAYcategories.zh_TWl10n to match actual sources.start_datefrom 1912 to 1998 as per our earliest available online document: Ministry of Interior (87) Order No. 8706459.Resolves #1912 .
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green