Merged
Conversation
Contributor
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRemoved an unused import and simplified Taiwan weekend detection for years ≤2000: instead of calling a helper, the code now computes the Saturday index as (dt.day - 1)//7 + 1 and treats 2nd and 4th Saturdays (with Sundays) as weekend days. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
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 #3077 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 306 306
Lines 18015 18016 +1
Branches 2285 2285
=========================================
+ Hits 18015 18016 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/taiwan.py(1 hunks)
🧰 Additional context used
🧠 Learnings (23)
📓 Common learnings
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2631
File: tests/countries/test_sint_maarten.py:62-0
Timestamp: 2025-06-14T21:12:07.224Z
Learning: KJhellico prefers to focus on completing and reviewing the main holiday implementation code before doing detailed reviews of the corresponding test files.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2349
File: tests/countries/test_taiwan.py:0-0
Timestamp: 2025-03-13T15:17:45.519Z
Learning: For Taiwan's holiday system, different categories (GOVERNMENT, OPTIONAL, SCHOOL, WORKDAY) have distinct uses and contexts, justifying separate instances rather than parameterization in tests.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: holidays/countries/south_africa.py:69-75
Timestamp: 2025-09-14T07:26:25.431Z
Learning: When reviewing historical holiday implementations in the vacanza/holidays repository, trust the maintainers' research and implementation decisions for specific historical edge cases, especially when they can provide sources like Wikipedia or other historical documentation that supports unusual or complex date calculation rules during specific time periods.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2874
File: tests/countries/test_taiwan.py:379-381
Timestamp: 2025-08-31T09:54:22.093Z
Learning: In Taiwan's holiday system, some holidays like Women's Day can appear in multiple categories simultaneously. Women's Day appears in the WORKDAY category for regular March 8th dates but also has special observed dates in the OPTIONAL category (1998-2000) when it was moved before Tomb-Sweeping Day. The test cases correctly reflect this dual-category behavior.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_albania.py:40-42
Timestamp: 2025-09-04T08:54:35.319Z
Learning: In the vacanza/holidays project test files, extract holiday name strings to local variables only when they are reused multiple times within the same test method (e.g., used in both assertHolidayName and assertNoHolidayName calls). When a holiday name is used only once, keep it inline rather than extracting it to a variable for the sake of consistency.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2777
File: holidays/countries/gambia.py:120-122
Timestamp: 2025-08-03T13:48:11.910Z
Learning: When reviewing holiday implementations in the holidays library, defer to the maintainers' choice of start years for specific holiday policies, as they likely have access to more reliable primary sources and official documentation than what can be found through web searches.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_french_polynesia.py:19-22
Timestamp: 2025-11-08T05:09:56.159Z
Learning: In the vacanza/holidays project's test framework (PR #2881 onwards), the base class CommonCountryTests provides a default test_no_holidays implementation that automatically tests years=start_year - 1 for standard PUBLIC category cases. Individual country test files should only override test_no_holidays when the country supports additional non-PUBLIC categories (GOVERNMENT, WORKDAY, OPTIONAL, etc.) with different start years requiring separate validation.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2323
File: holidays/countries/macau.py:278-377
Timestamp: 2025-03-05T02:35:03.298Z
Learning: For Macau holiday implementations, it's preferable to maintain separate methods for different holiday categories (MANDATORY, GOVERNMENT, PUBLIC) as they are based on different sets of laws, making the code easier to maintain despite having multiple year-based conditionals.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.641Z
Learning: Library-wide holiday patterns and their optimizations should be handled at the base class level (like InternationalHolidays) rather than documenting workarounds in individual country modules. This maintains separation of concerns and avoids documentation duplication.
Learnt from: Wasif-Shahzad
Repo: vacanza/holidays PR: 2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2654
File: holidays/countries/cabo_verde.py:133-141
Timestamp: 2025-07-14T20:23:48.198Z
Learning: The holidays library provides helper methods `_add_holiday_2nd_sun_of_may()` and `_add_holiday_3rd_sun_of_jun()` for adding holidays on the 2nd Sunday of May and 3rd Sunday of June respectively. These methods are used across multiple country implementations including Latvia, Finland, Belarus, Malaysia, Madagascar, and Cape Verde.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2874
File: tests/countries/test_taiwan.py:347-349
Timestamp: 2025-08-31T09:54:07.868Z
Learning: In Taiwan's holiday implementation, Children's Day (兒童節) appears in multiple categories simultaneously: Optional holidays (1998-2000 via special_optional_holidays_observed for observed date handling), Workday holidays (1998-2010), and Public holidays (2011+). The Optional category includes observed date logic for 1998-2000 that moves Children's Day to April 3rd when April 4th falls on Sunday.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2834
File: tests/financial/test_national_stock_exchange_of_india.py:342-360
Timestamp: 2025-08-30T12:52:58.539Z
Learning: In the NSE holidays implementation, assertLocalizedHolidays should only include holidays that are actually observed (trading holidays), not holidays that fall on weekends and are excluded by the observed_rule. For example, Eid al-Fitr 2023 falls on Saturday and is correctly excluded from localization tests.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:15-16
Timestamp: 2025-08-21T05:56:33.276Z
Learning: In the holidays library, when importing Gregorian month constants from holidays.calendars.gregorian, only import the months that are actually used in the date data. For example, if Islamic holiday dates only reference JUN, JUL, SEP, OCT, then only import those specific constants rather than importing additional unused months.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2945
File: tests/countries/test_sudan.py:128-146
Timestamp: 2025-09-18T17:51:08.767Z
Learning: Sudan's weekend rule changed on January 26, 2008: before this date, only Friday was a weekend day {FRI}, but from January 26, 2008 onwards, both Friday and Saturday became weekend days {FRI, SAT}. This explains why dates like "2008-01-19" (Saturday) are correctly non-weekend in tests before the rule change.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2945
File: tests/countries/test_sudan.py:128-146
Timestamp: 2025-09-18T17:51:08.767Z
Learning: Sudan's weekend rule changed on January 26, 2008: before this date, only Friday was a weekend day {FRI}, but from January 26, 2008 onwards, both Friday and Saturday became weekend days {FRI, SAT}. This explains why dates like "2008-01-19" (Saturday) are correctly non-weekend in tests before the rule change.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2854
File: tests/countries/test_sudan.py:21-25
Timestamp: 2025-09-10T21:12:39.614Z
Learning: Sudan holidays implementation does not include observed holiday logic (holidays that shift when falling on weekends), so test files for Sudan should not use years_non_observed parameter in the setUpClass method.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2653
File: holidays/locale/th/LC_MESSAGES/TW.po:17-21
Timestamp: 2025-06-21T18:06:50.027Z
Learning: KJhellico's username includes a tilde character (~) as part of their nickname (appears as "~Jhellico" in Last-Translator headers), which is intentional formatting and not an error.
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
Repo: vacanza/holidays PR: 2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-08-31T09:54:22.093Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2874
File: tests/countries/test_taiwan.py:379-381
Timestamp: 2025-08-31T09:54:22.093Z
Learning: In Taiwan's holiday system, some holidays like Women's Day can appear in multiple categories simultaneously. Women's Day appears in the WORKDAY category for regular March 8th dates but also has special observed dates in the OPTIONAL category (1998-2000) when it was moved before Tomb-Sweeping Day. The test cases correctly reflect this dual-category behavior.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-08-31T09:54:07.868Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2874
File: tests/countries/test_taiwan.py:347-349
Timestamp: 2025-08-31T09:54:07.868Z
Learning: In Taiwan's holiday implementation, Children's Day (兒童節) appears in multiple categories simultaneously: Optional holidays (1998-2000 via special_optional_holidays_observed for observed date handling), Workday holidays (1998-2010), and Public holidays (2011+). The Optional category includes observed date logic for 1998-2000 that moves Children's Day to April 3rd when April 4th falls on Sunday.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-03-13T15:17:45.519Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2349
File: tests/countries/test_taiwan.py:0-0
Timestamp: 2025-03-13T15:17:45.519Z
Learning: For Taiwan's holiday system, different categories (GOVERNMENT, OPTIONAL, SCHOOL, WORKDAY) have distinct uses and contexts, justifying separate instances rather than parameterization in tests.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-09-28T05:42:12.777Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 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:
holidays/countries/taiwan.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
Repo: vacanza/holidays PR: 2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants like FEB, TUE, MAR, and SUN internally through parent class implementations even when these constants don't appear directly in the file.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-08-31T09:54:07.868Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2874
File: tests/countries/test_taiwan.py:347-349
Timestamp: 2025-08-31T09:54:07.868Z
Learning: In Taiwan's holiday implementation, Children's Day (兒童節) appears in multiple categories simultaneously: Optional holidays (1998-2000), Workday holidays (1998-2010), and Public holidays (2011+). The Optional category includes observed date logic for 1998-2000 that moves Children's Day to April 3rd when April 4th falls on Sunday.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-07-14T20:23:48.198Z
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2654
File: holidays/countries/cabo_verde.py:133-141
Timestamp: 2025-07-14T20:23:48.198Z
Learning: The holidays library provides helper methods `_add_holiday_2nd_sun_of_may()` and `_add_holiday_3rd_sun_of_jun()` for adding holidays on the 2nd Sunday of May and 3rd Sunday of June respectively. These methods are used across multiple country implementations including Latvia, Finland, Belarus, Malaysia, Madagascar, and Cape Verde.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-07-09T20:27:37.760Z
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2623
File: holidays/countries/christmas_island.py:110-112
Timestamp: 2025-07-09T20:27:37.760Z
Learning: In Christmas Island, ANZAC Day (April 25) follows the same observed holiday rules as other holidays, using the SAT_SUN_TO_NEXT_MON rule to move to Monday when it falls on a weekend. The implementation correctly uses `_add_observed` wrapper around `_add_anzac_day`.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:15-16
Timestamp: 2025-08-21T05:56:33.276Z
Learning: In the holidays library, when importing Gregorian month constants from holidays.calendars.gregorian, only import the months that are actually used in the date data. For example, if Islamic holiday dates only reference JUN, JUL, SEP, OCT, then only import those specific constants rather than importing additional unused months.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-08-26T14:43:53.605Z
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2851
File: docs/holiday_categories.md:272-282
Timestamp: 2025-08-26T14:43:53.605Z
Learning: In the holidays library documentation, it's strongly advisable to recommend the use of constants from holidays.constants (e.g., PUBLIC, CATHOLIC) instead of direct string values when specifying holiday categories, as constants provide better type safety, IDE support, and prevent typos.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-07-10T03:36:16.461Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2706
File: holidays/countries/cayman_islands.py:80-97
Timestamp: 2025-07-10T03:36:16.461Z
Learning: In the holidays library, date dictionaries that map years to specific dates (like queens_birthday_dates, special holiday dates, etc.) are typically defined within the _populate_public_holidays method rather than as module-level constants. This is the established library-wide pattern and should be maintained for consistency.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-06-18T10:21:01.376Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2642
File: holidays/countries/france.py:300-319
Timestamp: 2025-06-18T10:21:01.376Z
Learning: In the France holidays implementation, legislative years for holiday changes should be hard-coded rather than extracted into constants, as this maintains consistency with the existing codebase pattern and provides historical accuracy for specific legislative acts.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-06-16T12:28:31.641Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.641Z
Learning: Library-wide holiday patterns and their optimizations should be handled at the base class level (like InternationalHolidays) rather than documenting workarounds in individual country modules. This maintains separation of concerns and avoids documentation duplication.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-04-23T14:55:35.504Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.504Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-07-10T03:36:16.461Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2706
File: holidays/countries/cayman_islands.py:80-97
Timestamp: 2025-07-10T03:36:16.461Z
Learning: In the holidays library, date dictionaries that map years to specific dates (like queens_birthday_dates, spring_bank_dates, thanksgiving_day_dates, etc.) are typically defined within the _populate_public_holidays method rather than as module-level constants. This is the established library-wide pattern seen across multiple country implementations including United Kingdom, United States, Sri Lanka, and others.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-03-23T10:11:50.465Z
Learnt from: KJhellico
Repo: vacanza/holidays PR: 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.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-03-04T11:32:45.095Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2323
File: holidays/countries/macau.py:407-461
Timestamp: 2025-03-04T11:32:45.095Z
Learning: In the holidays library, the standard approach for organizing static holidays is to use separate dictionaries for different categories (`government`, `mandatory`, and `public`), which are utilized by syntactic sugar methods.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-03-04T11:32:45.095Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2323
File: holidays/countries/macau.py:407-461
Timestamp: 2025-03-04T11:32:45.095Z
Learning: In the holidays library, the standard approach for organizing static holidays is to use separate dictionaries for different categories (like `special_government_holidays`, `special_mandatory_holidays`, and `special_public_holidays`), which are utilized by syntactic sugar methods to pick up the appropriate holidays based on the selected category.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-08-11T13:48:45.953Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2794
File: holidays/calendars/ethiopian.py:13-13
Timestamp: 2025-08-11T13:48:45.953Z
Learning: The holidays library does not use `__all__` declarations in calendar modules (holidays/calendars/). Calendar files follow a standard pattern of defining constants and functions directly without explicit exports, similar to the convention used in country modules.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-09-18T00:32:25.031Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2944
File: tests/countries/test_myanmar.py:32-39
Timestamp: 2025-09-18T00:32:25.031Z
Learning: Myanmar holidays implementation uses StaticHolidays with special_public_holidays for government-declared substitutions/bridging days, not ObservedHolidayBase for weekend-to-weekday shifting. Myanmar's substituted holidays are always present and don't have observed=False functionality.
Applied to files:
holidays/countries/taiwan.py
📚 Learning: 2025-09-14T16:23:46.707Z
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_botswana.py:59-59
Timestamp: 2025-09-14T16:23:46.707Z
Learning: In Botswana's holiday tests, assertNonObservedHoliday(dt) is used to verify that certain holidays (like Easter holidays) stay on their original dates regardless of the observed holiday system, which is different from assertNoNonObservedHoliday that checks for absence of non-observed holidays.
Applied to files:
holidays/countries/taiwan.py
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test Python 3.10 on windows-latest
- GitHub Check: Test Python 3.14 on windows-latest
- GitHub Check: Test Python 3.11 on windows-latest
PPsyrius
approved these changes
Nov 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Refactor Taiwan holidays (simplified calculation of 2nd and 4th Saturdays of the month).
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.