Update Thailand holidays: add support for historical weekend patterns #2951
Conversation
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
Summary by CodeRabbit
WalkthroughAdds a cached method to compute Buddhist Sabbath (Uposatha) dates for a Gregorian year, implements historical weekend rules for Thailand (including Sabbath inclusion in 1956–1957 and evolving weekend definitions), and adds tests validating the new calendar and weekend behaviors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.13.1)holidays/calendars/thai.py�[1;31mruff failed�[0m holidays/countries/thailand.py�[1;31mruff failed�[0m tests/countries/test_thailand.py�[1;31mruff failed�[0m 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 #2951 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 300 300
Lines 17919 17955 +36
Branches 2314 2326 +12
=========================================
+ Hits 17919 17955 +36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/calendars/thai.py(3 hunks)holidays/countries/thailand.py(2 hunks)tests/calendars/test_thai.py(2 hunks)tests/countries/test_thailand.py(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📚 Learning: 2025-06-15T11:50:44.366Z
Learnt from: PPsyrius
PR: vacanza/holidays#2601
File: holidays/countries/mongolia.py:140-146
Timestamp: 2025-06-15T11:50:44.366Z
Learning: The holidays library doesn't currently have helper functions like `_add_holiday_span` for adding consecutive multi-day holidays. Countries like Mongolia, Thailand, and Vietnam implement multi-day holiday spans using inline loops with `_timedelta(dt, delta)` pattern.
Applied to files:
holidays/countries/thailand.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-09-14T16:23:46.707Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.pytests/countries/test_thailand.py
📚 Learning: 2025-09-18T00:32:25.008Z
Learnt from: PPsyrius
PR: vacanza/holidays#2944
File: tests/countries/test_myanmar.py:32-39
Timestamp: 2025-09-18T00:32:25.008Z
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/thailand.py
📚 Learning: 2025-03-19T16:54:58.657Z
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-19T16:54:58.657Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.
Applied to files:
holidays/countries/thailand.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_thailand.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_thailand.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-06-25T10:36:39.909Z
Learnt from: PPsyrius
PR: vacanza/holidays#2678
File: tests/countries/test_united_states_virgin_islands.py:59-84
Timestamp: 2025-06-25T10:36:39.909Z
Learning: In the holidays library, test methods typically do not have docstrings. Only special test methods that need specific explanation (like edge cases or unique behaviors) have docstrings. Regular test methods like test_l10n_default, test_l10n_th, test_government_holidays, etc. should not have docstrings added.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-07-09T21:16:35.145Z
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: tests/countries/test_christmas_island.py:136-146
Timestamp: 2025-07-09T21:16:35.145Z
Learning: In Christmas Island's holiday implementation, the test_christmas_day method cannot use assertNoNonObservedHoliday because in some years observed Christmas Day overlaps with Boxing Day when both holidays are moved due to weekend conflicts, causing the standard non-observed holiday check to fail inappropriately.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-09-03T16:49:35.246Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_argentina.py:375-375
Timestamp: 2025-09-03T16:49:35.246Z
Learning: In the holidays library, national holiday tests use self.full_range (or similar comprehensive year ranges) even when explicit test dates only show modern observance. This is intentional for correctness and comprehensive coverage, unlike subdivision-specific holidays which have explicit year boundaries based on known start dates.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-09-10T21:12:39.614Z
Learnt from: KJhellico
PR: vacanza/holidays#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.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-08-30T12:52:58.539Z
Learnt from: KJhellico
PR: vacanza/holidays#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.
Applied to files:
tests/countries/test_thailand.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_thailand.py
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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:
tests/calendars/test_thai.py
🧬 Code graph analysis (4)
holidays/countries/thailand.py (3)
holidays/countries/algeria.py (1)
_get_weekend(71-79)holidays/holiday_base.py (1)
_get_weekend(844-845)holidays/calendars/thai.py (1)
buddhist_sabbath_dates(324-370)
tests/countries/test_thailand.py (3)
tests/countries/test_saudi_arabia.py (1)
test_weekend(164-181)tests/countries/test_malaysia.py (1)
test_weekend(958-1041)holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/calendars/test_thai.py (1)
holidays/calendars/thai.py (1)
buddhist_sabbath_dates(324-370)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
⏰ 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). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (5)
holidays/calendars/thai.py (2)
296-321: Good: caching _get_start_date.Memoizing the lunar-year start date is appropriate; the computation is deterministic and reused across methods.
Confirm that START_YEAR/END_YEAR bounds match the project’s supported range for Thai lunisolar calculations to avoid silent None on edge years.
14-14: No change required — project targets Python ≥ 3.9pyproject.toml declares requires-python = ">=3.9" and CI runs Python 3.13, so functools.cache is available and the compatibility shim is unnecessary.
holidays/countries/thailand.py (1)
29-31: LGTM on SAT/SUN imports.Keeps weekday constants local and avoids magic numbers.
tests/countries/test_thailand.py (1)
790-889: Strong coverage for historical weekend variants.The cases exercise: pre‑1939 (no weekend), 1939–1958 (Sun only, Sat half‑day), 1956–1957 (Buddhist Sabbath + Sun), and post‑1959 (Sat/Sun). Nicely targeted.
Consider adding one assertion just outside each boundary (e.g., 1959‑02‑28 and 1959‑03‑01) to pin the cutover.
tests/calendars/test_thai.py (1)
16-16: Month imports are fine here.The file uses nearly all month constants across tests; importing DEC is consistent with usage in the new test.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/calendars/test_thai.py (1)
22-25: Don’t call setUpClass() from setUp().Use setUp(), or drop the call entirely; unittest will invoke setUpClass() once per class.
Apply:
def setUp(self) -> None: - super().setUpClass() + super().setUp() self.calendar = calendars._ThaiLunisolar()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/calendars/thai.py(3 hunks)holidays/countries/thailand.py(2 hunks)tests/calendars/test_thai.py(2 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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:
tests/calendars/test_thai.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-06-15T11:52:39.572Z
Learnt from: PPsyrius
PR: vacanza/holidays#2601
File: tests/countries/test_mongolia.py:128-156
Timestamp: 2025-06-15T11:52:39.572Z
Learning: In the vacanza/holidays project tests, when testing holidays that span multiple consecutive days across many years (like Mongolia's National Festival spanning July 11-13), prefer explicit for loops over complex nested generator expressions with unpacking. The explicit loops are more readable, easier to maintain, and better communicate the testing intent even though the Big O complexity is equivalent.
Applied to files:
tests/calendars/test_thai.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/calendars/test_thai.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/calendars/test_thai.py
📚 Learning: 2025-09-14T17:17:14.387Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T16:05:55.205Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-09-14T16:23:46.707Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-09-18T00:32:25.008Z
Learnt from: PPsyrius
PR: vacanza/holidays#2944
File: tests/countries/test_myanmar.py:32-39
Timestamp: 2025-09-18T00:32:25.008Z
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/thailand.py
🧬 Code graph analysis (3)
tests/calendars/test_thai.py (1)
holidays/calendars/thai.py (1)
buddhist_sabbath_dates(324-370)
holidays/countries/thailand.py (2)
holidays/holiday_base.py (1)
_get_weekend(844-845)holidays/calendars/thai.py (1)
buddhist_sabbath_dates(324-370)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
🔇 Additional comments (7)
tests/calendars/test_thai.py (2)
17-17: Import extension is correct.DEC is used below; importing JAN–DEC matches usage and the “only import what you use” guidance.
315-319: Tuple comparison aligns with cached immutable return.Assertion matches buddhist_sabbath_dates() returning a tuple; good.
holidays/countries/thailand.py (2)
29-31: SAT/SUN import is appropriate.Needed for weekend computation; no issues.
204-221: Weekend logic reads correctly; optional inline citation.Semantics for 1956‑10‑01..1957‑10‑06 (Sabbath+Sun), 1939‑02‑28..1959‑02‑28 (Sun only), and 1959‑03‑01+ (Sat/Sun) look right. Consider a one‑liner with Gazette cutover dates to aid future readers.
Example:
elif date(1956, OCT, 1) <= dt <= date(1957, OCT, 6): - # Buddhist Sabbath Days and Sun (Full Day). + # Buddhist Sabbath Days and Sun (Full Day). Window per Gazette: + # starts 1956‑10‑01 (No. 9/2499), ends 1957‑10‑06 (No. 10/2500). weekend = {SUN}holidays/calendars/thai.py (3)
296-321: Good switch to @cache on _get_start_date().Per‑instance memoization is fine; domain is bounded (1914–2157).
323-371: Docstring says “list”; function returns tuple.Adjust wording to avoid confusion.
Apply:
def buddhist_sabbath_dates(self, year: int, calendar=None) -> tuple[date, ...]: @@ - Returns: - A list of `date` objects representing all Buddhist Sabbath days in the specified - Gregorian year. Returns an empty list if the year is outside the supported range. + Returns: + A tuple of `date` objects representing all Buddhist Sabbath days in the specified + Gregorian year. Returns an empty tuple if the year is outside the supported range.
14-15: OK to keep functools.cache — project targets Python 3.9+. pyproject.toml classifiers include "Programming Language :: Python :: 3.9" through 3.13 (no python_requires found); leave the import in holidays/calendars/thai.py.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/calendars/test_thai.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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:
tests/calendars/test_thai.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T16:05:55.205Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Applied to files:
tests/calendars/test_thai.py
🧬 Code graph analysis (1)
tests/calendars/test_thai.py (1)
holidays/calendars/thai.py (1)
buddhist_sabbath_dates(324-370)
⏰ 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). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (4)
tests/calendars/test_thai.py (4)
17-17: Import DEC is correct.DEC is used in the new Sabbath tests; import is justified.
210-262: Normal year sequence looks good.Dates read consistent with 29/30 alternation and filtering within Gregorian 2024.
263-315: Athikawan offsets align with extra‑day logic.The month‑7 extension (15‑waning present) appears consistent. No issues spotted.
316-319: Tuple equality assertion is appropriate.Type and order are enforced implicitly; good choice.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/calendars/test_thai.py (1)
22-25: Use super().setUp(), not setUpClass(), inside setUp.Calling the class-level hook in an instance setup is atypical and misleading. Use the instance hook.
Apply this diff:
- def setUp(self) -> None: - super().setUpClass() - self.calendar = calendars._ThaiLunisolar() + def setUp(self) -> None: + super().setUp() + self.calendar = calendars._ThaiLunisolar()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/calendars/thai.py(4 hunks)tests/calendars/test_thai.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T16:05:55.205Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Applied to files:
tests/calendars/test_thai.py
🧬 Code graph analysis (2)
tests/calendars/test_thai.py (1)
holidays/calendars/thai.py (1)
buddhist_sabbath_dates(326-368)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
🔇 Additional comments (5)
tests/calendars/test_thai.py (2)
17-17: Import scope is precise.DEC is used below in the Sabbath lists; aligned with the project’s “import only what you use” convention.
154-319: No duplicate test_buddhist_sabbath_dates found — approving.
Single definition present at tests/calendars/test_thai.py:154.holidays/calendars/thai.py (3)
298-324: Caching _get_start_date is appropriate.Per‑instance memoization eliminates repeated O(n) scans over year ranges. No issues spotted.
325-369: Docstring says “list”; function returns tuple — fix wording.Keep type docs consistent with the signature and return.
Apply this diff:
- Returns: - A list of `date` objects representing all Buddhist Sabbath days in the specified - Gregorian year. Returns an empty list if the year is outside the supported range. + Returns: + A tuple of `date` objects representing all Buddhist Sabbath days in the specified + Gregorian year. Returns an empty tuple if the year is outside the supported range.Also optional: consider inserting the Athikamat extra month explicitly “after Month 8” for readability, e.g., a brief comment or using slice assignment (
months[8:8] = [30]) to make intent unambiguous.
14-14: Confirm Python version support for functools.cacheholidays/calendars/thai.py:14 imports
from functools import cache. Repo search shows existing uses oflru_cacheand nopython_requiresor "Programming Language :: Python ::" classifiers — couldn't confirm the minimum Python. If the project must support older Pythons, replace withfunctools.lru_cache(maxsize=None)or gate the import by Python version; otherwise declare the minimum Python (addpython_requires/ trove classifiers).
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/calendars/thai.py(4 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In the Thailand holidays system, the difference between Thai and Khmer calendars is that certain holidays are observed one month later in Athikamat (Extra-Month) years for Thailand, not that the fundamental Buddhist Sabbath date calculations differ between calendar types.
📚 Learning: 2025-09-21T05:16:38.552Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-08-11T10:14:28.517Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.517Z
Learning: For Ethiopian holidays in the `holidays/groups/christian.py` file, docstring wording should maintain source-accurate phrasing (e.g., "in coincidence of" for Ethiopian New Year/Enkutatash), even when it might read awkwardly in English, to ensure consistency with official Ethiopian documentation.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-23T14:55:35.504Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/calendars/thai.py
📚 Learning: 2025-06-22T21:34:15.792Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:25-25
Timestamp: 2025-06-22T21:34:15.792Z
Learning: In the holidays project, Reference URLs in docstrings are kept as complete markdown links on single lines even when they exceed 100 characters, prioritizing reference readability over line length limits.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-08-11T10:14:14.399Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.399Z
Learning: For Ethiopian Orthodox Tewahedo Church references in code documentation, use "holiday" rather than "holy day" as this is the source-accurate terminology for Ethiopia.
Applied to files:
holidays/calendars/thai.py
🧬 Code graph analysis (1)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
🔇 Additional comments (3)
holidays/calendars/thai.py (3)
103-104: Docstring references: fine as-is.The previously broken
[text] (url)was corrected. Mixing plain angle-bracket links with a descriptive markdown link here is acceptable and used elsewhere.
298-324: Caching_get_start_dateis appropriate.Bounded domain (1914–2157) makes the unbounded cache safe; per‑instance caching via
selfis fine.
14-14: No change required — project requires Python ≥3.9pyproject.toml declares requires-python = ">=3.9" (classifiers list Python 3.9+), so functools.cache (added in Python 3.9) is supported and no lru_cache fallback is necessary.
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 (2)
holidays/calendars/thai.py(4 hunks)tests/calendars/test_thai.py(2 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In the Thailand holidays system, the difference between Thai and Khmer calendars is that certain holidays are observed one month later in Athikamat (Extra-Month) years for Thailand, not that the fundamental Buddhist Sabbath date calculations differ between calendar types.
📚 Learning: 2025-09-21T05:16:38.552Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Applied to files:
tests/calendars/test_thai.pyholidays/calendars/thai.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-21T05:16:38.552Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In the Thailand holidays system, the difference between Thai and Khmer calendars is that certain holidays are observed one month later in Athikamat (Extra-Month) years for Thailand, not that the fundamental Buddhist Sabbath date calculations differ between calendar types.
Applied to files:
tests/calendars/test_thai.pyholidays/calendars/thai.py
📚 Learning: 2025-09-14T07:26:25.431Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T17:17:14.387Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project, test methods that iterate over dictionaries of test data (like calendar drift tests) follow the established pattern without using unittest's subTest, as evidenced by the Thai calendar tests and maintained consistently across similar test structures.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-04-05T04:50:40.752Z
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.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T16:05:55.205Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-08-11T10:14:28.517Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.517Z
Learning: For Ethiopian holidays in the `holidays/groups/christian.py` file, docstring wording should maintain source-accurate phrasing (e.g., "in coincidence of" for Ethiopian New Year/Enkutatash), even when it might read awkwardly in English, to ensure consistency with official Ethiopian documentation.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-23T14:55:35.504Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/calendars/thai.py
📚 Learning: 2025-06-22T21:34:15.792Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:25-25
Timestamp: 2025-06-22T21:34:15.792Z
Learning: In the holidays project, Reference URLs in docstrings are kept as complete markdown links on single lines even when they exceed 100 characters, prioritizing reference readability over line length limits.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-08-11T10:14:14.399Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.399Z
Learning: For Ethiopian Orthodox Tewahedo Church references in code documentation, use "holiday" rather than "holy day" as this is the source-accurate terminology for Ethiopia.
Applied to files:
holidays/calendars/thai.py
🧬 Code graph analysis (2)
tests/calendars/test_thai.py (2)
tests/common.py (2)
TestCase(31-370)setUp(58-68)holidays/calendars/thai.py (1)
buddhist_sabbath_dates(326-368)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
⏰ 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). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (6)
holidays/calendars/thai.py (3)
103-104: References formatting looks good.The broken space in the markdown link is fixed; mixing angle-bracket and markdown styles is acceptable per project discussion.
298-324: Caching _get_start_date: OK.Per-instance cache is fine here; key space is small (year count). With the import shim above, this stays portable.
If you want cross-instance reuse, consider
@lru_cache(maxsize=None)on a@staticmethodthat takesyearonly; otherwise current approach is acceptable.
14-14: No shim required — project requires Python ≥3.9pyproject.toml declares requires-python = ">=3.9" (classifiers include 3.9–3.13) and tox targets python3.9+, so keep
from functools import cacheas-is.Likely an incorrect or invalid review comment.
tests/calendars/test_thai.py (3)
17-17: Import of DEC: OK.Used in expectations below; keeps constants style consistent.
23-24: setUp usage: OK.Instance‑level setup with
unittest.TestCaseis consistent and lightweight.
154-315: Great coverage for Sabbaths across year types.Solid golden‑path lists for Athikamat (2023), Pakatimat (2024), Athikawan (2025), plus out‑of‑range guards.
Consider one quick sanity assertion per year to harden ordering/count invariants without bloating fixtures:
result = self.calendar.buddhist_sabbath_dates(year) self.assertEqual(result, expected_dates) +self.assertTrue(all(a <= b for a, b in zip(result, result[1:]))) +self.assertGreater(len(result), 40)
KJhellico
reviewed
Sep 21, 2025
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 (3)
holidays/calendars/thai.py(4 hunks)holidays/countries/thailand.py(2 hunks)tests/calendars/test_thai.py(2 hunks)
🧰 Additional context used
🧠 Learnings (20)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In the Thailand holidays system, the difference between Thai and Khmer calendars is that certain holidays are observed one month later in Athikamat (Extra-Month) years for Thailand, not that the fundamental Buddhist Sabbath date calculations differ between calendar types.
📚 Learning: 2025-09-21T05:16:38.552Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Applied to files:
holidays/calendars/thai.pyholidays/countries/thailand.pytests/calendars/test_thai.py
📚 Learning: 2025-08-11T10:14:28.517Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.517Z
Learning: For Ethiopian holidays in the `holidays/groups/christian.py` file, docstring wording should maintain source-accurate phrasing (e.g., "in coincidence of" for Ethiopian New Year/Enkutatash), even when it might read awkwardly in English, to ensure consistency with official Ethiopian documentation.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-23T14:55:35.504Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/calendars/thai.py
📚 Learning: 2025-06-22T21:34:15.792Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:25-25
Timestamp: 2025-06-22T21:34:15.792Z
Learning: In the holidays project, Reference URLs in docstrings are kept as complete markdown links on single lines even when they exceed 100 characters, prioritizing reference readability over line length limits.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-08-11T10:14:14.399Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.399Z
Learning: For Ethiopian Orthodox Tewahedo Church references in code documentation, use "holiday" rather than "holy day" as this is the source-accurate terminology for Ethiopia.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-05-10T04:32:15.760Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:0-0
Timestamp: 2025-05-10T04:32:15.760Z
Learning: In the holidays package, detailed historical context and additional information should be added as comments at the method level or above conditional blocks, while comments directly above tr() function calls should only contain the holiday name itself (e.g., "# Independence Day.").
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-06-19T02:34:18.382Z
Learnt from: PPsyrius
PR: vacanza/holidays#2643
File: holidays/countries/mauritius.py:144-169
Timestamp: 2025-06-19T02:34:18.382Z
Learning: Custom holiday classes that extend _CustomHinduHolidays, _CustomIslamicHolidays, _CustomBuddhistHolidays, etc. in the holidays library do not use docstrings. They follow a pattern of using only inline comments above date dictionaries, as seen in Malaysia, Singapore, UAE, and other country implementations.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-09-17T15:53:16.940Z
Learnt from: PPsyrius
PR: vacanza/holidays#2944
File: holidays/countries/myanmar.py:108-111
Timestamp: 2025-09-17T15:53:16.940Z
Learning: In the holidays package, explanatory comments about year restrictions or policy context should be placed above conditional blocks or at method level, never directly above holiday name comments that precede tr() function calls, as this would include the explanatory text in .po localization files when running `make l10n`.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-03T12:36:41.201Z
Learnt from: PPsyrius
PR: vacanza/holidays#2398
File: holidays/countries/guinea.py:73-77
Timestamp: 2025-04-03T12:36:41.201Z
Learning: In the Holidays library, comments explaining year restrictions for holidays should be placed above the year check conditional statement, not inside it. Example format:
```python
# reason why goes here
if start_year <= self._year <= end_year:
# Holiday name
self._add_holiday_function(tr("Holiday Name"))
```
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-09-21T05:16:38.552Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.552Z
Learning: In the Thailand holidays system, the difference between Thai and Khmer calendars is that certain holidays are observed one month later in Athikamat (Extra-Month) years for Thailand, not that the fundamental Buddhist Sabbath date calculations differ between calendar types.
Applied to files:
holidays/calendars/thai.pytests/calendars/test_thai.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-09-14T16:23:46.707Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-09-18T00:32:25.008Z
Learnt from: PPsyrius
PR: vacanza/holidays#2944
File: tests/countries/test_myanmar.py:32-39
Timestamp: 2025-09-18T00:32:25.008Z
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/thailand.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T07:26:25.431Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T17:17:14.387Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project, test methods that iterate over dictionaries of test data (like calendar drift tests) follow the established pattern without using unittest's subTest, as evidenced by the Thai calendar tests and maintained consistently across similar test structures.
Applied to files:
tests/calendars/test_thai.py
📚 Learning: 2025-09-14T16:05:55.205Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Applied to files:
tests/calendars/test_thai.py
🧬 Code graph analysis (3)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
holidays/countries/thailand.py (2)
holidays/holiday_base.py (1)
_get_weekend(844-845)holidays/calendars/thai.py (1)
buddhist_sabbath_dates(326-368)
tests/calendars/test_thai.py (2)
tests/common.py (2)
TestCase(31-370)setUp(58-68)holidays/calendars/thai.py (1)
buddhist_sabbath_dates(326-368)
⏰ 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). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (6)
tests/calendars/test_thai.py (3)
16-19: Import extension is correct.Adding DEC is needed for later assertions; no issues.
22-25: setUp usage looks good.Fresh instance per test via super().setUp() is consistent with tests/common.py.
154-319: Excellent coverage for Buddhist Sabbath sequences.Strong, explicit spot‑checks for Athikamat/Pakatimat/Athikawan and boundary years; assertions compare sets, so order is irrelevant. LGTM.
holidays/calendars/thai.py (2)
298-324: Safe to cache: _get_start_date returns immutable date.@cache here is fine; pure and idempotent.
325-369: Return an immutable frozenset from buddhist_sabbath_dates.Cached mutable sets can be mutated by callers; return an immutable frozenset and update the typing. Repository search shows only membership/equality uses of this API (no callers mutate the returned collection), so the change is safe.
@@ -from typing import Optional +from typing import Optional, FrozenSet @@ - @cache - def buddhist_sabbath_dates(self, year: int) -> set[date]: + @cache + def buddhist_sabbath_dates(self, year: int) -> FrozenSet[date]: @@ - buddhist_sabbaths: set[date] = set() + buddhist_sabbaths: set[date] = set() @@ - return buddhist_sabbaths + return frozenset(buddhist_sabbaths)holidays/countries/thailand.py (1)
204-221: Weekend override logic matches the historical windows.Behavior is correct for 1939‑02‑28+, 1956‑10‑01..1957‑10‑06, and 1959‑03‑01+. Nice.
arkid15r
reviewed
Sep 27, 2025
arkid15r
approved these changes
Sep 27, 2025
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/calendars/thai.py(2 hunks)holidays/countries/thailand.py(2 hunks)tests/countries/test_thailand.py(1 hunks)
🧰 Additional context used
🧠 Learnings (32)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.578Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
📚 Learning: 2025-09-21T05:16:38.578Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.578Z
Learning: In holidays/calendars/thai.py, the `buddhist_sabbath_dates` method in `_ThaiLunisolar` is independent of calendar type (THAI_CALENDAR vs KHMER_CALENDAR) and calculates Buddhist Sabbath dates consistently regardless of which calendar is being used.
Applied to files:
tests/countries/test_thailand.pyholidays/calendars/thai.pyholidays/countries/thailand.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_thailand.py
📚 Learning: 2025-09-14T16:23:46.707Z
Learnt from: PPsyrius
PR: vacanza/holidays#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:
tests/countries/test_thailand.pyholidays/countries/thailand.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_thailand.py
📚 Learning: 2025-07-09T21:16:35.145Z
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: tests/countries/test_christmas_island.py:136-146
Timestamp: 2025-07-09T21:16:35.145Z
Learning: In Christmas Island's holiday implementation, the test_christmas_day method cannot use assertNoNonObservedHoliday because in some years observed Christmas Day overlaps with Boxing Day when both holidays are moved due to weekend conflicts, causing the standard non-observed holiday check to fail inappropriately.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-09-03T16:49:35.246Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_argentina.py:375-375
Timestamp: 2025-09-03T16:49:35.246Z
Learning: In the holidays library, national holiday tests use self.full_range (or similar comprehensive year ranges) even when explicit test dates only show modern observance. This is intentional for correctness and comprehensive coverage, unlike subdivision-specific holidays which have explicit year boundaries based on known start dates.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-09-10T21:12:39.614Z
Learnt from: KJhellico
PR: vacanza/holidays#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.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/countries/test_thailand.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_thailand.py
📚 Learning: 2025-08-30T12:52:58.539Z
Learnt from: KJhellico
PR: vacanza/holidays#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.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-09-14T16:05:55.205Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Applied to files:
tests/countries/test_thailand.py
📚 Learning: 2025-08-11T10:14:28.517Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.517Z
Learning: For Ethiopian holidays in the `holidays/groups/christian.py` file, docstring wording should maintain source-accurate phrasing (e.g., "in coincidence of" for Ethiopian New Year/Enkutatash), even when it might read awkwardly in English, to ensure consistency with official Ethiopian documentation.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-23T14:55:35.504Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/calendars/thai.py
📚 Learning: 2025-06-22T21:34:15.792Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:25-25
Timestamp: 2025-06-22T21:34:15.792Z
Learning: In the holidays project, Reference URLs in docstrings are kept as complete markdown links on single lines even when they exceed 100 characters, prioritizing reference readability over line length limits.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-08-11T10:14:14.399Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.399Z
Learning: For Ethiopian Orthodox Tewahedo Church references in code documentation, use "holiday" rather than "holy day" as this is the source-accurate terminology for Ethiopia.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-05-10T04:32:15.760Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:0-0
Timestamp: 2025-05-10T04:32:15.760Z
Learning: In the holidays package, detailed historical context and additional information should be added as comments at the method level or above conditional blocks, while comments directly above tr() function calls should only contain the holiday name itself (e.g., "# Independence Day.").
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-06-19T02:34:18.382Z
Learnt from: PPsyrius
PR: vacanza/holidays#2643
File: holidays/countries/mauritius.py:144-169
Timestamp: 2025-06-19T02:34:18.382Z
Learning: Custom holiday classes that extend _CustomHinduHolidays, _CustomIslamicHolidays, _CustomBuddhistHolidays, etc. in the holidays library do not use docstrings. They follow a pattern of using only inline comments above date dictionaries, as seen in Malaysia, Singapore, UAE, and other country implementations.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-09-17T15:53:16.940Z
Learnt from: PPsyrius
PR: vacanza/holidays#2944
File: holidays/countries/myanmar.py:108-111
Timestamp: 2025-09-17T15:53:16.940Z
Learning: In the holidays package, explanatory comments about year restrictions or policy context should be placed above conditional blocks or at method level, never directly above holiday name comments that precede tr() function calls, as this would include the explanatory text in .po localization files when running `make l10n`.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-03T12:36:41.201Z
Learnt from: PPsyrius
PR: vacanza/holidays#2398
File: holidays/countries/guinea.py:73-77
Timestamp: 2025-04-03T12:36:41.201Z
Learning: In the Holidays library, comments explaining year restrictions for holidays should be placed above the year check conditional statement, not inside it. Example format:
```python
# reason why goes here
if start_year <= self._year <= end_year:
# Holiday name
self._add_holiday_function(tr("Holiday Name"))
```
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-09-21T05:16:38.578Z
Learnt from: PPsyrius
PR: vacanza/holidays#2951
File: tests/calendars/test_thai.py:0-0
Timestamp: 2025-09-21T05:16:38.578Z
Learning: In the Thailand holidays system, the difference between Thai and Khmer calendars is that certain holidays are observed one month later in Athikamat (Extra-Month) years for Thailand, not that the fundamental Buddhist Sabbath date calculations differ between calendar types.
Applied to files:
holidays/calendars/thai.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-06-16T12:28:31.641Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-08-03T13:48:11.910Z
Learnt from: KJhellico
PR: vacanza/holidays#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.
Applied to files:
holidays/countries/thailand.py
📚 Learning: 2025-09-14T07:26:25.431Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/countries/thailand.py
📚 Learning: 2025-09-14T17:17:14.387Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.
Applied to files:
holidays/countries/thailand.py
📚 Learning: 2025-07-10T03:36:16.461Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-06-18T10:21:01.376Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-09-18T07:01:12.236Z
Learnt from: PPsyrius
PR: vacanza/holidays#2942
File: holidays/countries/south_africa.py:91-98
Timestamp: 2025-09-18T07:01:12.236Z
Learning: In the holidays library South Africa implementation, inline ternary operators for holiday name selection (like "Republic Day" if self._year >= 1961 else "Union Day") are intentionally kept inline rather than extracted to separate variables, as this structure is designed in preparation for future localization (l10n) support where the contextual relationship between conditions and translatable strings needs to be preserved.
Applied to files:
holidays/countries/thailand.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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/thailand.py
📚 Learning: 2025-08-25T04:28:02.061Z
Learnt from: PPsyrius
PR: vacanza/holidays#2848
File: tests/countries/test_somalia.py:44-127
Timestamp: 2025-08-25T04:28:02.061Z
Learning: In the holidays library, Islamic holiday tests use `self.no_estimated_holidays = Country(years=years, islamic_show_estimated=False)` as the library-wide standard approach to simplify test cases. This pattern is intentional and preferred over testing estimated labels.
Applied to files:
holidays/countries/thailand.py
📚 Learning: 2025-09-18T00:32:25.031Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/thailand.py
🧬 Code graph analysis (3)
tests/countries/test_thailand.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
holidays/calendars/thai.py (1)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)
holidays/countries/thailand.py (3)
holidays/countries/taiwan.py (1)
_get_weekend(99-112)holidays/holiday_base.py (1)
_get_weekend(844-845)holidays/calendars/thai.py (1)
buddhist_sabbath_dates(326-368)
⏰ 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). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (4)
tests/countries/test_thailand.py (1)
849-888: Half‑day Saturdays correctly excluded.This block looks accurate and aligns with historical half‑day behavior.
holidays/countries/thailand.py (1)
29-31: Importing SAT/SUN is correct and consistent with other country modules.holidays/calendars/thai.py (2)
103-105: References formatting change looks fine.The earlier spacing issue is resolved; no further action.
325-369: buddhist_sabbath_dates: solid, cached, and handles Athikamat/Athikawan correctly.Logic and year‑boundary handling look good.
Comment on lines
+204
to
+221
| def _get_weekend(self, dt: date) -> set[int]: | ||
| if dt >= date(1959, MAR, 1): | ||
| # SAT & SUN (Full Day). | ||
| weekend = {SAT, SUN} | ||
| elif date(1956, OCT, 1) <= dt <= date(1957, OCT, 6): | ||
| # Buddhist Sabbath Days and Sun (Full Day). | ||
| weekend = {SUN} | ||
| buddhist_sabbath_dates = self._thai_calendar.buddhist_sabbath_dates(dt.year) | ||
| if dt in buddhist_sabbath_dates: | ||
| weekend.add(dt.weekday()) | ||
| elif dt >= date(1939, FEB, 28): | ||
| # SAT from 12:00 onwards and SUN (Full Day). | ||
| weekend = {SUN} | ||
| else: | ||
| # Prior to this, there was no concept of weekend. | ||
| weekend = set() | ||
|
|
||
| return weekend |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Weekend logic matches legal history; minor readability tweak.
Rename the local variable to avoid shadowing the method name.
- buddhist_sabbath_dates = self._thai_calendar.buddhist_sabbath_dates(dt.year)
- if dt in buddhist_sabbath_dates:
+ sabbaths = self._thai_calendar.buddhist_sabbath_dates(dt.year)
+ if dt in sabbaths:
weekend.add(dt.weekday())Ensure tests cover both ends of the 1956-10-01 to 1957-10-06 window (inclusive) for sabbath and non‑sabbath weekdays.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _get_weekend(self, dt: date) -> set[int]: | |
| if dt >= date(1959, MAR, 1): | |
| # SAT & SUN (Full Day). | |
| weekend = {SAT, SUN} | |
| elif date(1956, OCT, 1) <= dt <= date(1957, OCT, 6): | |
| # Buddhist Sabbath Days and Sun (Full Day). | |
| weekend = {SUN} | |
| buddhist_sabbath_dates = self._thai_calendar.buddhist_sabbath_dates(dt.year) | |
| if dt in buddhist_sabbath_dates: | |
| weekend.add(dt.weekday()) | |
| elif dt >= date(1939, FEB, 28): | |
| # SAT from 12:00 onwards and SUN (Full Day). | |
| weekend = {SUN} | |
| else: | |
| # Prior to this, there was no concept of weekend. | |
| weekend = set() | |
| return weekend | |
| def _get_weekend(self, dt: date) -> set[int]: | |
| if dt >= date(1959, MAR, 1): | |
| # SAT & SUN (Full Day). | |
| weekend = {SAT, SUN} | |
| elif date(1956, OCT, 1) <= dt <= date(1957, OCT, 6): | |
| # Buddhist Sabbath Days and Sun (Full Day). | |
| weekend = {SUN} | |
| sabbaths = self._thai_calendar.buddhist_sabbath_dates(dt.year) | |
| if dt in sabbaths: | |
| weekend.add(dt.weekday()) | |
| elif dt >= date(1939, FEB, 28): | |
| # SAT from 12:00 onwards and SUN (Full Day). | |
| weekend = {SUN} | |
| else: | |
| # Prior to this, there was no concept of weekend. | |
| weekend = set() | |
| return weekend |
🤖 Prompt for AI Agents
In holidays/countries/thailand.py around lines 204 to 221, the local variable
weekend should be renamed to avoid shadowing the method name; change all uses of
the variable to weekend_days (assign initial sets, add weekday when appropriate,
and return weekend_days), keeping the logic identical; update any references in
this function to use weekend_days and run/update tests to ensure coverage for
both 1956-10-01 and 1957-10-06 (inclusive) for sabbath and non-sabbath weekdays
per the reviewer request.
Comment on lines
+791
to
+847
| for dt in ( | ||
| "1940-01-07", # SUN. | ||
| "1940-01-14", # SUN. | ||
| "1940-01-21", # SUN. | ||
| "1940-01-28", # SUN. | ||
| "1956-09-02", # SUN. | ||
| "1956-09-09", # SUN. | ||
| "1956-09-16", # SUN. | ||
| "1956-09-23", # SUN. | ||
| "1956-09-30", # SUN. | ||
| "1956-10-04", # THU (Buddhist Sabbath Day). | ||
| "1956-10-07", # SUN. | ||
| "1956-10-12", # FRI (Buddhist Sabbath Day). | ||
| "1956-10-14", # SUN. | ||
| "1956-10-19", # FRI (Buddhist Sabbath Day). | ||
| "1956-10-21", # SUN. | ||
| "1956-10-27", # SAT (Buddhist Sabbath Day). | ||
| "1956-10-28", # SUN. | ||
| "1957-01-06", # SUN. | ||
| "1957-01-08", # TUE (Buddhist Sabbath Day). | ||
| "1957-01-13", # SUN. | ||
| "1957-01-15", # TUE (Buddhist Sabbath Day). | ||
| "1957-01-20", # SUN. | ||
| "1957-01-23", # WED (Buddhist Sabbath Day). | ||
| "1957-01-27", # SUN. | ||
| "1957-01-30", # WED (Buddhist Sabbath Day). | ||
| "1957-09-01", # SUN. | ||
| "1957-09-02", # MON (Buddhist Sabbath Day). | ||
| "1957-09-08", # SUN. | ||
| "1957-09-09", # MON (Buddhist Sabbath Day). | ||
| "1957-09-15", # SUN. | ||
| "1957-09-17", # TUE (Buddhist Sabbath Day). | ||
| "1957-09-22", # SUN. | ||
| "1957-09-24", # TUE (Buddhist Sabbath Day). | ||
| "1957-09-29", # SUN. | ||
| "1957-10-02", # WED (Buddhist Sabbath Day). | ||
| "1957-10-06", # SUN. | ||
| "1957-10-13", # SUN. | ||
| "1957-10-20", # SUN. | ||
| "1957-10-27", # SUN. | ||
| "1958-01-05", # SUN. | ||
| "1958-01-12", # SUN. | ||
| "1958-01-19", # SUN. | ||
| "1958-01-26", # SUN. | ||
| "1960-01-02", # SAT. | ||
| "1960-01-03", # SUN. | ||
| "1960-01-09", # SAT. | ||
| "1960-01-10", # SUN. | ||
| "1960-01-16", # SAT. | ||
| "1960-01-17", # SUN. | ||
| "1960-01-23", # SAT. | ||
| "1960-01-24", # SUN. | ||
| "1960-01-30", # SAT. | ||
| "1960-01-31", # SUN. | ||
| ): | ||
| self.assertTrue(self.holidays.is_weekend(dt)) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Weekend truth cases cover key regimes well. Add cutover edge checks.
Consider adding a few boundary asserts to harden against off‑by‑one regressions:
- 1956-10-01 (Mon) -> False
- 1957-10-07 (Mon) -> False
- 1959-02-28 (Sat) -> False, 1959-03-07 (Sat) -> True
🤖 Prompt for AI Agents
In tests/countries/test_thailand.py around lines 791-847, add boundary
assertions for the weekend checker to catch off-by-one regressions: assert that
"1956-10-01" (Mon) is not a weekend, assert that "1957-10-07" (Mon) is not a
weekend, assert that "1959-02-28" (Sat) is not a weekend (per the requested
expectation) and assert that "1959-03-07" (Sat) is a weekend; add these
immediately before or after the existing loop of dates so they run as individual
truth/false checks against self.holidays.is_weekend.
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
buddhist_sabbath_datesmethod to retrieve all Buddhist Sabbath dates in a given Gregorian year (applicable for Cambodia, Laos, and Thailand).Resolves #2353 .
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.