Add variable weekend support #2945
Merged
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 date-aware weekend hook to HolidayBase and replaces per-year in-method weekend mutations with per-date Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
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.0)holidays/countries/malaysia.py�[1;31mruff failed�[0m holidays/countries/united_arab_emirates.py�[1;31mruff failed�[0m Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 #2945 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 296 296
Lines 17620 17652 +32
Branches 2270 2271 +1
=========================================
+ Hits 17620 17652 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
holidays/countries/malaysia.py (1)
81-83: Docstring contradicts implementation (and phrasing)
- Text says “Friday to Sunday” and “Saturday to Sunday”; code implements Fri–Sat for Johor/Kedah/Kelantan/Terengganu and Sat–Sun elsewhere.
- In Johor (until 1994 and in 2014-2024) and Kedah it's Friday to Sunday, - in Kelantan and Terengganu - Saturday to Sunday. + Weekends: Friday–Saturday in Johor (until 1994 and 2014–2024), Kedah, + Kelantan, and Terengganu; Saturday–Sunday elsewhere.holidays/countries/sudan.py (1)
82-89: Arafah Day mislabeled as “Eid al‑Adha”.
User‑facing name should be “وقفة عرفة”.Apply:
- name = tr("عيد الأضحى المبارك") - self._add_arafah_day(name) + name = tr("عيد الأضحى المبارك") + self._add_arafah_day(tr("وقفة عرفة"))holidays/countries/united_arab_emirates.py (1)
60-68: Add an explicit class‑level weekend default for consistency.
Most providers in this PR declareweekendat class scope. It also protects any legacy code that might still readself.weekend.Apply:
# Founded on DEC 2, 1971. start_year = 1972 + weekend = {SAT, SUN}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (21)
holidays/countries/algeria.py(3 hunks)holidays/countries/jordan.py(3 hunks)holidays/countries/kuwait.py(3 hunks)holidays/countries/malaysia.py(2 hunks)holidays/countries/oman.py(3 hunks)holidays/countries/qatar.py(3 hunks)holidays/countries/saudi_arabia.py(3 hunks)holidays/countries/sudan.py(3 hunks)holidays/countries/united_arab_emirates.py(2 hunks)holidays/countries/yemen.py(3 hunks)holidays/holiday_base.py(3 hunks)tests/countries/test_algeria.py(1 hunks)tests/countries/test_jordan.py(1 hunks)tests/countries/test_kuwait.py(1 hunks)tests/countries/test_malaysia.py(1 hunks)tests/countries/test_oman.py(1 hunks)tests/countries/test_qatar.py(1 hunks)tests/countries/test_saudi_arabia.py(1 hunks)tests/countries/test_sudan.py(1 hunks)tests/countries/test_united_arab_emirates.py(1 hunks)tests/countries/test_yemen.py(1 hunks)
🧰 Additional context used
🧠 Learnings (52)
📓 Common learnings
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.
📚 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_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_kuwait.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_jordan.pytests/countries/test_qatar.pytests/countries/test_malaysia.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_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_kuwait.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_qatar.pytests/countries/test_malaysia.pyholidays/countries/sudan.pytests/countries/test_oman.py
📚 Learning: 2025-09-18T03:19:23.712Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_algeria.py:28-30
Timestamp: 2025-09-18T03:19:23.712Z
Learning: In the vacanza/holidays project, tests now use self.start_year and self.end_year from the TestCase class instead of country-specific aliases (like DZ.start_year) for start_year and end_year references. This approach provides the test framework with better control over test year ranges rather than being tied to specific country start years.
Applied to files:
tests/countries/test_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_jordan.pytests/countries/test_algeria.pytests/countries/test_qatar.pytests/countries/test_malaysia.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_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_kuwait.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_jordan.pytests/countries/test_algeria.pytests/countries/test_qatar.pytests/countries/test_malaysia.pyholidays/countries/saudi_arabia.pytests/countries/test_oman.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: The method assertNonObservedHoliday(dt) is a valid assertion method in the holidays test framework that verifies holidays occur on their original (non-observed) dates, which is different from assertNoNonObservedHoliday that checks for absence of non-observed holidays. It's used to test that certain holidays stay on fixed dates regardless of observed holiday rules.
Applied to files:
tests/countries/test_saudi_arabia.pytests/countries/test_oman.py
📚 Learning: 2025-04-04T10:52:41.546Z
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: holidays/countries/guinea.py:106-110
Timestamp: 2025-04-04T10:52:41.546Z
Learning: In the Guinea holidays implementation, observed Eid al-Fitr cases are properly covered by the test_eid_al_fitr_day() method, which tests both the regular holiday dates and the observed dates when the holiday falls on a non-working day (for years >= 2023).
Applied to files:
tests/countries/test_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_kuwait.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_qatar.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_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_kuwait.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_jordan.pytests/countries/test_algeria.pytests/countries/test_qatar.pytests/countries/test_malaysia.pytests/countries/test_oman.py
📚 Learning: 2025-09-03T18:29:09.398Z
Learnt from: KJhellico
PR: vacanza/holidays#2820
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:76-76
Timestamp: 2025-09-03T18:29:09.398Z
Learning: The assertNoNonObservedHoliday method in tests/common.py accepts a holidays object as its first parameter, followed by the dates to check. Usage like assertNoNonObservedHoliday(self.government_holidays_non_observed, obs_dt) is correct and intended behavior.
Applied to files:
tests/countries/test_saudi_arabia.pytests/countries/test_oman.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_saudi_arabia.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_malaysia.pytests/countries/test_oman.py
📚 Learning: 2025-04-04T10:52:41.546Z
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: holidays/countries/guinea.py:106-110
Timestamp: 2025-04-04T10:52:41.546Z
Learning: In the Guinea holidays implementation, observed Eid al-Fitr cases are covered by the test_eid_al_fitr_day() method, which tests both regular holiday dates and the observed dates when the holiday falls on a non-working day (for years >= 2023).
Applied to files:
tests/countries/test_saudi_arabia.pytests/countries/test_yemen.pytests/countries/test_kuwait.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_qatar.py
📚 Learning: 2025-07-02T18:17:53.342Z
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:17:53.342Z
Learning: In the Saint Vincent and the Grenadines holidays implementation, New Year's Day is added without observed rules using `_add_new_years_day()` and should not include observed rule testing in its test method. Only holidays explicitly wrapped with `_add_observed()` have observed behavior.
Applied to files:
tests/countries/test_yemen.pytests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_jordan.py
📚 Learning: 2025-09-10T14:35:54.603Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_brazil.py:28-30
Timestamp: 2025-09-10T14:35:54.603Z
Learning: In the holidays project, the test_no_holidays method should test ALL supported_categories (via categories=CountryClass.supported_categories) rather than just the default PUBLIC category, to ensure comprehensive validation that no holidays exist before start_year across any supported category including OPTIONAL and subdivision categories.
Applied to files:
tests/countries/test_yemen.pytests/countries/test_sudan.pytests/countries/test_qatar.pytests/countries/test_malaysia.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_yemen.py
📚 Learning: 2025-04-05T04:33:53.254Z
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
Applied to files:
tests/countries/test_kuwait.py
📚 Learning: 2025-04-05T06:49:06.217Z
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
Applied to files:
tests/countries/test_united_arab_emirates.pytests/countries/test_sudan.pytests/countries/test_jordan.pytests/countries/test_malaysia.py
📚 Learning: 2025-09-12T21:37:10.710Z
Learnt from: KJhellico
PR: vacanza/holidays#2854
File: tests/countries/test_sudan.py:29-31
Timestamp: 2025-09-12T21:37:10.710Z
Learning: For countries in the holidays library that only use the default PUBLIC category (like Sudan), there's no need to specify `categories=Sudan.supported_categories` in test_no_holidays methods, as it would be equivalent to the default behavior.
Applied to files:
tests/countries/test_sudan.py
📚 Learning: 2025-08-25T22:13:30.310Z
Learnt from: KJhellico
PR: vacanza/holidays#2854
File: README.md:1557-1562
Timestamp: 2025-08-25T22:13:30.310Z
Learning: Sudan holidays implementation inherits from IslamicHolidays but does not expose an ISLAMIC category in its supported_categories attribute. The Supported Categories column in README.md should remain blank for Sudan.
Applied to files:
tests/countries/test_sudan.pyholidays/countries/sudan.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_sudan.py
📚 Learning: 2025-09-14T16:03:13.558Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_barbados.py:21-23
Timestamp: 2025-09-14T16:03:13.558Z
Learning: In tests/countries/test_barbados.py, using years_non_observed=range(2000, 2024) is intentional because all observed holiday examples fall within 2001-2023, making this range appropriate for limiting testing to years where observed holidays actually exist in the test data.
Applied to files:
tests/countries/test_sudan.py
📚 Learning: 2025-06-15T15:24:53.055Z
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: holidays/countries/faroe_islands.py:62-67
Timestamp: 2025-06-15T15:24:53.055Z
Learning: The `HolidayBase` class uses `__getattr__` to dynamically implement `_add_holiday_*` methods through pattern matching, including patterns like `_add_holiday_<n>_days_past_easter`, `_add_holiday_<month>_<day>`, and various weekday-relative patterns. Methods like `_add_holiday_26_days_past_easter` are not explicitly defined but are dynamically generated when called.
Applied to files:
holidays/holiday_base.py
📚 Learning: 2025-08-24T13:01:51.370Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2852
File: tests/countries/test_tajikistan.py:26-28
Timestamp: 2025-08-24T13:01:51.370Z
Learning: In the holidays project country test files, the standard method name for testing country aliases (ISO codes) is `test_country_aliases`, not `test_aliases`. This naming convention is consistently used across all country test files.
Applied to files:
tests/countries/test_algeria.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_malaysia.py
📚 Learning: 2025-04-13T19:10:31.502Z
Learnt from: KJhellico
PR: vacanza/holidays#2465
File: holidays/countries/suriname.py:219-251
Timestamp: 2025-04-13T19:10:31.502Z
Learning: The `_CustomIslamicHolidays` classes in this project contain only exact verified holiday dates from reliable sources, rather than calculated or estimated future dates. This is by design to ensure accuracy, particularly for religious holidays that may follow lunar calendars or depend on local observations.
Applied to files:
holidays/countries/yemen.pyholidays/countries/united_arab_emirates.pyholidays/countries/saudi_arabia.pyholidays/countries/algeria.pyholidays/countries/oman.pyholidays/countries/qatar.pyholidays/countries/kuwait.pyholidays/countries/sudan.pyholidays/countries/jordan.py
📚 Learning: 2025-06-19T02:34:14.456Z
Learnt from: PPsyrius
PR: vacanza/holidays#2643
File: holidays/countries/mauritius.py:171-184
Timestamp: 2025-06-19T02:34:14.456Z
Learning: In the holidays library, `_CustomIslamicHolidays` subclasses follow a consistent pattern of NOT having docstrings. They go directly to defining date dictionaries, as evidenced by Malaysia, Singapore, UAE, and dozens of other country implementations.
Applied to files:
holidays/countries/yemen.pyholidays/countries/united_arab_emirates.pyholidays/countries/saudi_arabia.pyholidays/countries/oman.pyholidays/countries/qatar.pyholidays/countries/sudan.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/countries/yemen.pyholidays/countries/united_arab_emirates.pyholidays/countries/qatar.pyholidays/countries/sudan.py
📚 Learning: 2025-05-06T13:01:22.693Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2522
File: holidays/countries/yemen.py:158-163
Timestamp: 2025-05-06T13:01:22.693Z
Learning: In the holidays library, the RAMADAN_BEGINNING_DATES dictionary in country-specific Islamic holiday classes (like YemenIslamicHolidays) is used indirectly through the backend. When a country class calls _add_holiday_29_ramadan(), the IslamicHolidays implementation uses the country's custom calendar dates to calculate the 29th day of Ramadan by adding 28 days to the beginning date.
Applied to files:
holidays/countries/yemen.pyholidays/countries/united_arab_emirates.pyholidays/countries/qatar.pyholidays/countries/sudan.py
📚 Learning: 2025-06-13T12:18:03.539Z
Learnt from: PPsyrius
PR: vacanza/holidays#2614
File: holidays/countries/guyana.py:78-90
Timestamp: 2025-06-13T12:18:03.539Z
Learning: The holidays codebase now uses the constructor signature pattern `__init__(self, *args, islamic_show_estimated: bool = True, **kwargs)` across country classes.
Applied to files:
holidays/countries/yemen.pyholidays/countries/algeria.pyholidays/countries/qatar.pyholidays/countries/kuwait.pyholidays/countries/sudan.pyholidays/countries/jordan.py
📚 Learning: 2025-07-10T15:55:34.523Z
Learnt from: KJhellico
PR: vacanza/holidays#2706
File: holidays/countries/cayman_islands.py:50-55
Timestamp: 2025-07-10T15:55:34.523Z
Learning: The `islamic_show_estimated` parameter in country class constructors is only needed for countries that implement Islamic holidays (inherit from IslamicHolidays or _CustomIslamicHolidays groups). Countries with only Christian and secular holidays do not need this parameter.
Applied to files:
holidays/countries/yemen.pyholidays/countries/algeria.pyholidays/countries/oman.pyholidays/countries/qatar.pyholidays/countries/kuwait.pyholidays/countries/sudan.pyholidays/countries/jordan.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/yemen.pyholidays/countries/sudan.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:
holidays/countries/saudi_arabia.pyholidays/countries/oman.pyholidays/countries/kuwait.pyholidays/countries/sudan.py
📚 Learning: 2025-08-21T04:51:16.449Z
Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:13-13
Timestamp: 2025-08-21T04:51:16.449Z
Learning: In the holidays library, country classes with localization support consistently use `from gettext import gettext as tr` import and wrap class-level attributes like `estimated_label`, `observed_label`, and `observed_estimated_label` with `tr()` calls. This is the standard library-wide practice for l10n-enabled entities and is required for proper string extraction when generating .po files.
Applied to files:
holidays/countries/saudi_arabia.py
📚 Learning: 2025-08-21T04:56:03.780Z
Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
Learning: In the holidays library, countries with localization support consistently use tr() wrappers around holiday names when calling _add_* methods (e.g., self._add_new_years_day(tr("Holiday Name"))). This is the established pattern across United States, Thailand, and other l10n-enabled countries, contrary to any suggestion that translation is handled internally by _add_* methods.
Applied to files:
holidays/countries/saudi_arabia.pyholidays/countries/malaysia.pyholidays/countries/jordan.py
📚 Learning: 2025-08-26T20:10:05.288Z
Learnt from: KJhellico
PR: vacanza/holidays#2834
File: holidays/financial/national_stock_exchange_of_india.py:38-44
Timestamp: 2025-08-26T20:10:05.288Z
Learning: For National Stock Exchange of India (NSE) holidays implementation, only `estimated_label = tr("%s (estimated)")` is needed for localization support. The `observed_label` and `observed_estimated_label` are not required for this financial market holidays implementation.
Applied to files:
holidays/countries/saudi_arabia.pyholidays/countries/oman.pyholidays/countries/kuwait.pyholidays/countries/jordan.py
📚 Learning: 2025-08-22T19:06:04.303Z
Learnt from: KJhellico
PR: vacanza/holidays#2850
File: holidays/countries/christmas_island.py:75-80
Timestamp: 2025-08-22T19:06:04.303Z
Learning: Christmas Island's docstring for the `islamic_show_estimated` parameter follows the exact same format used consistently across all countries with Islamic holidays in the codebase: "Whether to add 'estimated' label to Islamic holidays name if holiday date is estimated." This is the standard, established pattern and should not be changed.
Applied to files:
holidays/countries/saudi_arabia.pyholidays/countries/algeria.pyholidays/countries/oman.pyholidays/countries/kuwait.pyholidays/countries/sudan.pyholidays/countries/jordan.py
📚 Learning: 2025-09-14T04:36:25.108Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: holidays/countries/chile.py:121-123
Timestamp: 2025-09-14T04:36:25.108Z
Learning: In the Chile holidays implementation, the MON_ONLY observance rule correctly implements Law 20.983 for January 2. MON_ONLY is defined as ObservedRule({TUE: None, WED: None, THU: None, FRI: None, SAT: None, SUN: None}), which means when _add_observed() is called with MON_ONLY, it removes holidays that fall on Tuesday through Sunday (via self.pop(dt)) and only keeps holidays that fall on Monday. This perfectly implements the law that January 2 is only a holiday when January 1 falls on Sunday.
Applied to files:
holidays/countries/saudi_arabia.pyholidays/countries/malaysia.py
📚 Learning: 2025-03-23T10:11:50.465Z
Learnt from: KJhellico
PR: vacanza/holidays#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/saudi_arabia.pyholidays/countries/malaysia.py
📚 Learning: 2025-08-26T21:24:41.827Z
Learnt from: KJhellico
PR: vacanza/holidays#2860
File: holidays/countries/burkina_faso.py:27-30
Timestamp: 2025-08-26T21:24:41.827Z
Learning: Countries in the holidays library that don't have localization support yet should use plain English strings for labels (e.g., `estimated_label = "%s (estimated)"`), while only countries with existing .po translation files should use `tr()` wrapping. Check for the presence of .po files in holidays/locale to determine if a country has localization support.
Applied to files:
holidays/countries/malaysia.py
📚 Learning: 2025-08-21T04:56:03.780Z
Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
Learning: In the holidays library, countries with localization support DO use tr() wrappers around holiday names when calling _add_* methods. This is the correct pattern for l10n-enabled country implementations, contrary to previous learning about translation being handled internally by _add_* methods.
Applied to files:
holidays/countries/malaysia.py
📚 Learning: 2025-06-16T14:08:09.492Z
Learnt from: KJhellico
PR: vacanza/holidays#2635
File: holidays/countries/bhutan.py:13-16
Timestamp: 2025-06-16T14:08:09.492Z
Learning: In the holidays library, translation is performed in the `_add_holiday` methods, not at the string level. Holiday strings passed to `_add_holiday_*` methods are processed through the translation machinery within those methods, so using `tr()` or `self.tr()` wrappers is unnecessary.
Applied to files:
holidays/countries/malaysia.py
📚 Learning: 2025-05-10T04:02:13.815Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:249-253
Timestamp: 2025-05-10T04:02:13.815Z
Learning: Holiday name comments directly above tr() function calls in the holidays package should only contain the holiday name itself (e.g., "# Independence Day.") without any additional context, dates, or historical information.
Applied to files:
holidays/countries/malaysia.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, 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/malaysia.py
📚 Learning: 2025-09-03T17:11:54.474Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: holidays/countries/algeria.py:16-22
Timestamp: 2025-09-03T17:11:54.474Z
Learning: In Algeria's holidays implementation, Islamic holidays (like Eid al-Fitr, Eid al-Adha, Mawlid, etc.) are categorized as PUBLIC holidays rather than ISLAMIC category holidays, since they are national public holidays for all citizens. The country inherits from IslamicHolidays but only implements _populate_public_holidays() method, with all Islamic holidays added there instead of having a separate _populate_islamic_holidays() method.
Applied to files:
holidays/countries/algeria.pyholidays/countries/kuwait.pyholidays/countries/jordan.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/algeria.pyholidays/countries/kuwait.pyholidays/countries/jordan.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/algeria.pyholidays/countries/kuwait.pyholidays/countries/sudan.pyholidays/countries/jordan.py
📚 Learning: 2025-06-29T10:36:06.138Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:51-108
Timestamp: 2025-06-29T10:36:06.138Z
Learning: There is no project-wide convention in the holidays library to organize holidays by calendar type (Islamic holidays first, then Gregorian holidays). Countries organize holidays in various ways - often chronologically, by importance, or by logical grouping - and Islamic holidays are frequently placed at the end of the _populate_public_holidays method rather than at the beginning.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-08-19T21:00:47.849Z
Learnt from: KJhellico
PR: vacanza/holidays#2831
File: holidays/countries/south_sudan.py:84-88
Timestamp: 2025-08-19T21:00:47.849Z
Learning: In the holidays library, Islamic holidays use dedicated methods for additional days (like `_add_eid_al_fitr_day_two`, `_add_eid_al_adha_day_two`) rather than parameters. The methods don't accept a `days` parameter - each day has its own specific method.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-08-28T02:34:11.657Z
Learnt from: PPsyrius
PR: vacanza/holidays#2860
File: holidays/groups/eastern.py:36-36
Timestamp: 2025-08-28T02:34:11.657Z
Learning: The default estimated_label change from "%s (estimated)" to "%s" in holidays/groups/eastern.py is intentional to remove English-only fallback text from localized holiday names. Countries that need estimated labels should define them properly with localization support rather than relying on the English fallback.
Applied to files:
holidays/countries/kuwait.pyholidays/countries/jordan.py
📚 Learning: 2025-08-26T02:36:50.853Z
Learnt from: PPsyrius
PR: vacanza/holidays#2854
File: holidays/countries/sudan.py:28-39
Timestamp: 2025-08-26T02:36:50.853Z
Learning: Sudan's official holiday observance policy is documented by the Sudan Embassy: holidays falling on Saturday are observed on the preceding Friday, and holidays falling on Sunday are observed on the following Monday. However, specific legal authority or enactment date for this policy could not be verified from available sources.
Applied to files:
holidays/countries/sudan.py
📚 Learning: 2025-08-19T20:39:52.024Z
Learnt from: KJhellico
PR: vacanza/holidays#2831
File: holidays/countries/south_sudan.py:84-88
Timestamp: 2025-08-19T20:39:52.024Z
Learning: In South Sudan holidays implementation, Eid al-Fitr and Eid al-Adha follow a specific two-day pattern: the first day of each Eid is a PUBLIC holiday (national holiday for everyone), while the second day is an ISLAMIC category holiday (Muslim-specific only). This should be implemented by keeping first day calls in _populate_public_holidays() and adding second day calls in _populate_islamic_holidays() method.
Applied to files:
holidays/countries/sudan.py
📚 Learning: 2025-08-19T20:36:15.300Z
Learnt from: KJhellico
PR: vacanza/holidays#2831
File: holidays/countries/south_sudan.py:84-88
Timestamp: 2025-08-19T20:36:15.300Z
Learning: In South Sudan holidays implementation, Eid al-Fitr and Eid al-Adha holidays should use the ISLAMIC category system. The first day of each Eid is a national public holiday (appears in both PUBLIC and ISLAMIC categories), while additional days are typically Muslim-specific and should only appear in the ISLAMIC category through the _populate_islamic_holidays() method.
Applied to files:
holidays/countries/sudan.py
📚 Learning: 2025-03-30T20:18:46.006Z
Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:24-26
Timestamp: 2025-03-30T20:18:46.006Z
Learning: In the holidays library, country classes do not directly implement `_populate()`. Instead, they implement specialized methods like `_populate_public_holidays()`, and the base class `HolidayBase` handles the orchestration by calling these specialized methods.
Applied to files:
holidays/countries/sudan.py
🧬 Code graph analysis (21)
tests/countries/test_saudi_arabia.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_yemen.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_kuwait.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_united_arab_emirates.py (2)
tests/countries/test_qatar.py (1)
test_weekend(163-179)holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_sudan.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_jordan.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
holidays/holiday_base.py (10)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/malaysia.py (1)
_get_weekend(167-173)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/qatar.py (1)
_get_weekend(56-58)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/countries/sudan.py (1)
_get_weekend(47-50)holidays/countries/united_arab_emirates.py (1)
_get_weekend(83-86)holidays/countries/yemen.py (1)
_get_weekend(65-67)
tests/countries/test_algeria.py (2)
tests/countries/test_jordan.py (1)
test_weekend(46-61)holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_qatar.py (2)
tests/countries/test_kuwait.py (1)
test_weekend(45-61)holidays/holiday_base.py (1)
is_weekend(1141-1151)
tests/countries/test_malaysia.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
holidays/countries/yemen.py (6)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/united_arab_emirates.py (9)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/qatar.py (1)
_get_weekend(56-58)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/countries/sudan.py (1)
_get_weekend(47-50)holidays/countries/yemen.py (1)
_get_weekend(65-67)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/saudi_arabia.py (6)
holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/yemen.py (1)
_get_weekend(65-67)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/malaysia.py (4)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/united_arab_emirates.py (1)
_get_weekend(83-86)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/algeria.py (10)
holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/malaysia.py (1)
_get_weekend(167-173)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/qatar.py (1)
_get_weekend(56-58)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/countries/sudan.py (1)
_get_weekend(47-50)holidays/countries/united_arab_emirates.py (1)
_get_weekend(83-86)holidays/countries/yemen.py (1)
_get_weekend(65-67)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/oman.py (4)
holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/qatar.py (3)
holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/kuwait.py (8)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/qatar.py (1)
_get_weekend(56-58)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/countries/united_arab_emirates.py (1)
_get_weekend(83-86)holidays/countries/yemen.py (1)
_get_weekend(65-67)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/sudan.py (3)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/jordan.py (7)
holidays/countries/algeria.py (1)
_get_weekend(66-74)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/oman.py (1)
_get_weekend(68-70)holidays/countries/qatar.py (1)
_get_weekend(56-58)holidays/countries/saudi_arabia.py (1)
_get_weekend(75-77)holidays/countries/yemen.py (1)
_get_weekend(65-67)holidays/holiday_base.py (1)
_get_weekend(844-845)
tests/countries/test_oman.py (1)
holidays/holiday_base.py (1)
is_weekend(1141-1151)
⏰ 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). (2)
- GitHub Check: Test Python 3.13 on windows-latest
- GitHub Check: Test Python 3.11 on windows-latest
🔇 Additional comments (41)
holidays/holiday_base.py (3)
873-876: Delegation to overridable weekend logic is correct.
_is_weekend()now respects per‑date overrides. LGTM.
1141-1152: Public API stays stable while using the new core.
is_weekend()delegating through__keytransform__→_is_weekend()is clean and backward compatible.
844-846: Return a read-only weekend set; verify no direct mutations remainholidays/holiday_base.py:844-846 — Return an immutable/abstract set to avoid exposing a mutable class attribute (e.g.,
return frozenset(self.weekend)and annotate as-> collections.abc.AbstractSet[int]).Verification inconclusive: the provided ripgrep produced no output; re-run locally
rg -nP 'self\.weekend\s*='andrg -nP '\bin\s+self\.weekend\b'and confirm no callers assign/modifyself.weekend.holidays/countries/yemen.py (3)
13-13: Needed import for date-aware weekend logic.All good.
49-49: Explicit default weekend improves clarity.Good default; subclasses can still override via
_get_weekend.
65-68: Correct weekend switch on 2013-08-15.Boundary handling (Thu just before, Fri/Sat after) matches the tests and new base behavior.
tests/countries/test_yemen.py (1)
253-269: Great boundary coverage for variable weekends.Covers pre/post switch and the boundary date explicitly. Nice.
tests/countries/test_kuwait.py (1)
45-62: Accurate assertions across the 2007-09-01 switch.Good mix of before/after cases.
tests/countries/test_algeria.py (1)
280-304: Thorough multi-era checks (pre-1976, 1976–2008, 2009+).Solid validation of all three weekend regimes.
tests/countries/test_united_arab_emirates.py (1)
145-159: Covers 2022 policy change cleanly.Pre- and post-2022 assertions look correct.
tests/countries/test_jordan.py (1)
46-61: Good check around the 2000-01-06 threshold.True/false sets align with THU–FRI → FRI–SAT transition.
tests/countries/test_saudi_arabia.py (1)
164-181: Covers pre/post 2013-06-28 with boundary dates included.Matches SA weekend shift semantics.
holidays/countries/malaysia.py (2)
167-174: Good: date-aware weekend centralizationAdding _get_weekend(dt) keeps weekend rules pure and stateless. Matches the new base hook.
167-174: Confirmed — no change requiredJohor's official weekend switches to Saturday–Sunday effective 1 January 2025; the code's Fri–Sat upper bound (<=2024) is correct.
tests/countries/test_malaysia.py (1)
1016-1031: Fix duplicate date; Johor weekend reversion confirmed (effective January 1, 2025)Replace the duplicate "2024-12-27" with "2024-12-28" (SAT). Johor did revert to Saturday–Sunday effective January 1, 2025, so the 2025 SAT/SUN expectations are correct.
"2014-01-03", # FRI. "2014-01-04", # SAT. "2024-12-27", # FRI. - "2024-12-27", # SAT. + "2024-12-28", # SAT. "2025-01-04", # SAT. "2025-01-05", # SUN.holidays/countries/kuwait.py (4)
13-16: Imports are lean and correct.Only needed month/weekday constants are imported; SEP is used in the boundary check. Good.
35-35: Set sensible default weekend.Class-level default matches the modern FRI–SAT weekend.
48-52: Weekend switch date logic LGTM.Boundary at 2007-09-01 with SEP constant is precise; source cited in comment. Ensure tests cover 2007-08-31 and 2007-09-01 (they appear to).
53-56: Holiday l10n usage consistent.tr() wrappers are applied; no issues spotted.
holidays/countries/qatar.py (4)
13-13: Type import added appropriately.Needed for _get_weekend signature; fine.
40-40: Default weekend set matches post-2003 switch.Keeps current default aligned with modern practice.
56-59: Date-aware weekend switch looks correct.Aug 1, 2003 boundary is encoded cleanly.
Add/confirm tests on 2003‑07‑31, 2003‑08‑01, 2003‑08‑02 to assert is_weekend correctness on both sides.
60-68: Use of weekday helpers is idiomatic - correct assessment confirmed.The verification confirms FEB and MAR constants are properly imported in
holidays.calendars.gregorianand used throughout the codebase. The helper methods like_add_holiday_2nd_tue_of_feband_add_holiday_1st_sun_of_marare part of the framework's design pattern and handle calendar constants internally through parent class implementations.Qatar's weekend tests exist at line 163 in
tests/countries/test_qatar.py, validating the weekend configuration changes from THU-FRI to FRI-SAT on August 1, 2003.holidays/countries/saudi_arabia.py (3)
17-17: Import set is minimal and used.JUN for the switch date; SEP/NOV used in data; OK.
49-49: Default weekend set applied.Matches contemporary FRI–SAT weekend.
75-78: Weekend switch date encoded correctly.June 28, 2013 boundary is reflected accurately.
holidays/countries/jordan.py (4)
16-16: Imports are tight.Only JAN and weekday constants used by the boundary; good.
34-34: Default weekend set aligns with present-day practice.
48-51: Weekend switch boundary LGTM.Jan 6, 2000 boundary with citation; clear and consistent with the PR pattern.
53-65: Holiday population uses tr() consistently.No issues spotted.
holidays/countries/oman.py (4)
16-31: Month constants import set looks justified.All imported months appear in OmanIslamicHolidays date maps; no unnecessary extras.
54-54: Default weekend set matches modern practice.
68-71: Weekend switch date logic LGTM.May 1, 2013 boundary encoded with MAY constant; clear.
Confirm tests include dates immediately before and after 2013‑05‑01 (looks covered).
100-108: Nice touch on 29 Ramadan handling.Eid eve added only when not auto-added by contiguous dates; clean guard.
holidays/countries/sudan.py (2)
38-38: Class-level weekend default looks good.
Matches the new base API and keeps current-era default explicit.
47-51: Pre‑2008 weekend likely not Friday‑only — please verify.
Many regional peers were THU–FRI before switching. If Sudan was THU–FRI prior to 2008‑01‑26, adjust accordingly.If confirmed, apply:
-from holidays.calendars.gregorian import JAN, MAY, JUL, AUG, SEP, FRI, SAT +from holidays.calendars.gregorian import JAN, MAY, JUL, AUG, SEP, THU, FRI, SAT - return {FRI, SAT} if dt >= date(2008, JAN, 26) else {FRI} + return {FRI, SAT} if dt >= date(2008, JAN, 26) else {THU, FRI}⛔ Skipped due to learnings
Learnt from: PPsyrius PR: vacanza/holidays#2854 File: holidays/countries/sudan.py:28-39 Timestamp: 2025-08-26T02:36:50.853Z Learning: Sudan's official holiday observance policy is documented by the Sudan Embassy: holidays falling on Saturday are observed on the preceding Friday, and holidays falling on Sunday are observed on the following Monday. However, specific legal authority or enactment date for this policy could not be verified from available sources.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.holidays/countries/united_arab_emirates.py (2)
83-87: Weekend logic LGTM.
Date split for 2022 switch is correct and readable.
88-137: Holiday population unchanged — looks solid.
No behavioral regressions spotted.holidays/countries/algeria.py (3)
51-51: Class-level weekend default — good addition.
Aligns with the new_get_weekend(dt)contract.
66-74: Weekend timeline reads correct and is cleanly expressed.
Three-era split (pre‑1976 SAT–SUN, 1976–2008 THU–FRI, 2009+ FRI–SAT) is clear.
76-127: Public holidays population LGTM.
Names, thresholds, and legal references match the comments.
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 (2)
holidays/countries/algeria.py(4 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_brazil.py:28-30
Timestamp: 2025-09-10T14:35:54.603Z
Learning: In the holidays project, the test_no_holidays method should test ALL supported_categories (via categories=CountryClass.supported_categories) rather than just the default PUBLIC category, to ensure comprehensive validation that no holidays exist before start_year across any supported category including OPTIONAL and subdivision categories.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: holidays/countries/faroe_islands.py:62-67
Timestamp: 2025-06-15T15:24:53.055Z
Learning: The `HolidayBase` class uses `__getattr__` to dynamically implement `_add_holiday_*` methods through pattern matching, including patterns like `_add_holiday_<n>_days_past_easter`, `_add_holiday_<month>_<day>`, and various weekday-relative patterns. Methods like `_add_holiday_26_days_past_easter` are not explicitly defined but are dynamically generated when called.
📚 Learning: 2025-09-03T17:11:54.474Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: holidays/countries/algeria.py:16-22
Timestamp: 2025-09-03T17:11:54.474Z
Learning: In Algeria's holidays implementation, Islamic holidays (like Eid al-Fitr, Eid al-Adha, Mawlid, etc.) are categorized as PUBLIC holidays rather than ISLAMIC category holidays, since they are national public holidays for all citizens. The country inherits from IslamicHolidays but only implements _populate_public_holidays() method, with all Islamic holidays added there instead of having a separate _populate_islamic_holidays() method.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-06-13T12:18:03.539Z
Learnt from: PPsyrius
PR: vacanza/holidays#2614
File: holidays/countries/guyana.py:78-90
Timestamp: 2025-06-13T12:18:03.539Z
Learning: The holidays codebase now uses the constructor signature pattern `__init__(self, *args, islamic_show_estimated: bool = True, **kwargs)` across country classes.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-03-04T11:32:45.095Z
Learnt from: PPsyrius
PR: vacanza/holidays#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/algeria.py
📚 Learning: 2025-09-16T04:11:33.513Z
Learnt from: PPsyrius
PR: vacanza/holidays#2928
File: holidays/countries/algeria.py:16-22
Timestamp: 2025-09-16T04:11:33.513Z
Learning: In the holidays library, country class base inheritance follows alphabetical ordering after HolidayBase. For example, Algeria correctly uses: HolidayBase, ChristianHolidays, HebrewCalendarHolidays, InternationalHolidays, IslamicHolidays (alphabetical after the required HolidayBase first position).
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-08-26T21:29:47.753Z
Learnt from: KJhellico
PR: vacanza/holidays#2860
File: tests/common.py:365-372
Timestamp: 2025-08-26T21:29:47.753Z
Learning: In the holidays library, countries with Islamic holidays inherit directly from the IslamicHolidays class (e.g., `class Afghanistan(HolidayBase, InternationalHolidays, IslamicHolidays)`). The separate `_CustomIslamicHolidays` classes (like `AfghanistanIslamicHolidays`) are helper classes for specific date data, not the main country classes. Therefore, `isinstance(holidays_instance, IslamicHolidays)` is sufficient to detect all countries with Islamic holidays.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-04-23T09:59:19.886Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/countries/ethiopia.py:45-45
Timestamp: 2025-04-23T09:59:19.886Z
Learning: For the Ethiopia holidays class, it's appropriate to add a return type hint only to the `_is_leap_year` method to match the base class implementation in `holidays/holiday_base.py`, while keeping other methods without type hints to maintain consistency with other country implementations.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-07-10T15:55:34.523Z
Learnt from: KJhellico
PR: vacanza/holidays#2706
File: holidays/countries/cayman_islands.py:50-55
Timestamp: 2025-07-10T15:55:34.523Z
Learning: The `islamic_show_estimated` parameter in country class constructors is only needed for countries that implement Islamic holidays (inherit from IslamicHolidays or _CustomIslamicHolidays groups). Countries with only Christian and secular holidays do not need this parameter.
Applied to files:
holidays/countries/algeria.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/algeria.py
📚 Learning: 2025-04-13T19:10:31.502Z
Learnt from: KJhellico
PR: vacanza/holidays#2465
File: holidays/countries/suriname.py:219-251
Timestamp: 2025-04-13T19:10:31.502Z
Learning: The `_CustomIslamicHolidays` classes in this project contain only exact verified holiday dates from reliable sources, rather than calculated or estimated future dates. This is by design to ensure accuracy, particularly for religious holidays that may follow lunar calendars or depend on local observations.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-08-22T19:06:04.303Z
Learnt from: KJhellico
PR: vacanza/holidays#2850
File: holidays/countries/christmas_island.py:75-80
Timestamp: 2025-08-22T19:06:04.303Z
Learning: Christmas Island's docstring for the `islamic_show_estimated` parameter follows the exact same format used consistently across all countries with Islamic holidays in the codebase: "Whether to add 'estimated' label to Islamic holidays name if holiday date is estimated." This is the standard, established pattern and should not be changed.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-06-29T10:36:06.138Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:51-108
Timestamp: 2025-06-29T10:36:06.138Z
Learning: There is no project-wide convention in the holidays library to organize holidays by calendar type (Islamic holidays first, then Gregorian holidays). Countries organize holidays in various ways - often chronologically, by importance, or by logical grouping - and Islamic holidays are frequently placed at the end of the _populate_public_holidays method rather than at the beginning.
Applied to files:
holidays/countries/algeria.py
📚 Learning: 2025-08-19T21:00:47.849Z
Learnt from: KJhellico
PR: vacanza/holidays#2831
File: holidays/countries/south_sudan.py:84-88
Timestamp: 2025-08-19T21:00:47.849Z
Learning: In the holidays library, Islamic holidays use dedicated methods for additional days (like `_add_eid_al_fitr_day_two`, `_add_eid_al_adha_day_two`) rather than parameters. The methods don't accept a `days` parameter - each day has its own specific method.
Applied to files:
holidays/countries/algeria.py
🧬 Code graph analysis (1)
holidays/countries/algeria.py (4)
holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/countries/united_arab_emirates.py (1)
_get_weekend(83-86)holidays/holiday_base.py (1)
_get_weekend(844-845)
⏰ 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). (2)
- GitHub Check: Test Python 3.14 on windows-latest
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (4)
holidays/countries/algeria.py (4)
13-14: Good: future annotations enabled.Keeps runtime clean and supports built-in generics in annotations.
16-30: TYPE_CHECKING gate is fine.Lightweight type-only import; aligns with the new coverage exclusion.
56-56: Default weekend set appropriately.Post‑2009 default matches most recent practice; base hook will handle older years.
81-132: Confirm statute-effective years for multi‑day Eids and naming change.Double‑check:
- Eid al‑Fitr day three from 2024 vs law date (Loi n° 23‑10, 1444 AH/2023 CE).
- Eid al‑Adha day two from 1969 while ordinance is 1968; ensure 1969 is the first effective observance year.
- Independence Day naming switch in 2005.
If sources differ, annotate with a brief comment and adjust year thresholds.
arkid15r
reviewed
Sep 18, 2025
There was a problem hiding this comment.
Looks good, great improvement 👍
I've been thinking about introducing TYPE_CHECKING for a while. We don't currently use it, but I'm pretty sure there are plenty of imports that aren't needed at runtime.
Given the number of date imports used only for type hints, now seems like a good time to start using it. It's not urgent, so we can always clean it up later, but if you'd like to address it in this PR, you can use algeria.py as a reference.
|
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/countries/malaysia.py(3 hunks)holidays/countries/united_arab_emirates.py(3 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
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.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: holidays/countries/faroe_islands.py:62-67
Timestamp: 2025-06-15T15:24:53.055Z
Learning: The `HolidayBase` class uses `__getattr__` to dynamically implement `_add_holiday_*` methods through pattern matching, including patterns like `_add_holiday_<n>_days_past_easter`, `_add_holiday_<month>_<day>`, and various weekday-relative patterns. Methods like `_add_holiday_26_days_past_easter` are not explicitly defined but are dynamically generated when called.
Learnt from: KJhellico
PR: vacanza/holidays#2829
File: tests/countries/test_canada.py:291-296
Timestamp: 2025-08-19T19:47:21.735Z
Learning: In the holidays library, HolidayBase objects automatically populate years on-demand when expand=True (the default). When checking dates from years not initially in the years range, those years are automatically populated via the logic at line 646 in holiday_base.py: "if self.expand and dt.year not in self.years:". This means tests can check dates outside the initial year range without needing separate holiday instances.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_algeria.py:28-30
Timestamp: 2025-09-18T03:19:23.712Z
Learning: In the vacanza/holidays project, tests now use self.start_year and self.end_year from the TestCase class instead of country-specific aliases (like DZ.start_year) for start_year and end_year references. This approach provides the test framework with better control over test year ranges rather than being tied to specific country start years.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_brazil.py:28-30
Timestamp: 2025-09-10T14:35:54.603Z
Learning: In the holidays project, the test_no_holidays method should test ALL supported_categories (via categories=CountryClass.supported_categories) rather than just the default PUBLIC category, to ensure comprehensive validation that no holidays exist before start_year across any supported category including OPTIONAL and subdivision categories.
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.
📚 Learning: 2025-04-13T19:10:31.502Z
Learnt from: KJhellico
PR: vacanza/holidays#2465
File: holidays/countries/suriname.py:219-251
Timestamp: 2025-04-13T19:10:31.502Z
Learning: The `_CustomIslamicHolidays` classes in this project contain only exact verified holiday dates from reliable sources, rather than calculated or estimated future dates. This is by design to ensure accuracy, particularly for religious holidays that may follow lunar calendars or depend on local observations.
Applied to files:
holidays/countries/united_arab_emirates.py
📚 Learning: 2025-06-19T02:34:14.456Z
Learnt from: PPsyrius
PR: vacanza/holidays#2643
File: holidays/countries/mauritius.py:171-184
Timestamp: 2025-06-19T02:34:14.456Z
Learning: In the holidays library, `_CustomIslamicHolidays` subclasses follow a consistent pattern of NOT having docstrings. They go directly to defining date dictionaries, as evidenced by Malaysia, Singapore, UAE, and dozens of other country implementations.
Applied to files:
holidays/countries/united_arab_emirates.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/countries/united_arab_emirates.py
📚 Learning: 2025-03-23T10:11:50.465Z
Learnt from: KJhellico
PR: vacanza/holidays#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/malaysia.py
📚 Learning: 2025-09-14T04:36:25.108Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: holidays/countries/chile.py:121-123
Timestamp: 2025-09-14T04:36:25.108Z
Learning: In the Chile holidays implementation, the MON_ONLY observance rule correctly implements Law 20.983 for January 2. MON_ONLY is defined as ObservedRule({TUE: None, WED: None, THU: None, FRI: None, SAT: None, SUN: None}), which means when _add_observed() is called with MON_ONLY, it removes holidays that fall on Tuesday through Sunday (via self.pop(dt)) and only keeps holidays that fall on Monday. This perfectly implements the law that January 2 is only a holiday when January 1 falls on Sunday.
Applied to files:
holidays/countries/malaysia.py
🧬 Code graph analysis (2)
holidays/countries/united_arab_emirates.py (4)
holidays/countries/malaysia.py (1)
_get_weekend(172-178)holidays/countries/jordan.py (1)
_get_weekend(48-51)holidays/countries/kuwait.py (1)
_get_weekend(48-51)holidays/holiday_base.py (1)
_get_weekend(844-845)
holidays/countries/malaysia.py (2)
holidays/countries/united_arab_emirates.py (1)
_get_weekend(88-91)holidays/holiday_base.py (1)
_get_weekend(844-845)
⏰ 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: Build distribution
🔇 Additional comments (6)
holidays/countries/malaysia.py (2)
13-14: Postponed annotations import: good.This aligns with the repo’s typing pattern and avoids runtime import cycles.
16-16: TYPE_CHECKING gate for date: consistent.Kept type-only import; runtime uses dt.year so no NameError risks.
Also applies to: 57-59
holidays/countries/united_arab_emirates.py (4)
13-14: Postponed annotations import: good.Matches project convention.
16-16: TYPE_CHECKING gate for date: fine.No runtime dependency on datetime; annotations stay light.
Also applies to: 38-40
88-92: Weekend rule looks correct.UAE moved to Sat–Sun from 2022; condition captures it cleanly. (channelnewsasia.com)
93-96: Method reordering only.No functional change in _populate_public_holidays.
|
I added TYPE_CHECKING for the countries involved in this PR. I think it would be better to deal with rest of countries in separate PR. |
arkid15r
reviewed
Sep 20, 2025
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
Add variable weekend support (for countries where the weekend has changed over the years).
HolidayBase::_get_weekendmethod has been added, which can be overridden in the country class with custom rules implementation.Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.