Update Ethiopia holidays, add ETHIOPIAN_CALENDAR support, Julian Date Drift adjustment pre-1899 and post-2099
#2794
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
…ChristianHolidays`
Summary by CodeRabbit
WalkthroughAdds Ethiopian calendar support and leap-year helper, introduces Julian→Gregorian drift helper, updates Christian and International holiday computations to be drift- and Ethiopian-calendar-aware, updates Ethiopia country holiday data (Islamic dates for 2025), adds tests, and updates ET snapshot entries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #2794 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 284 285 +1
Lines 17049 17066 +17
Branches 2253 2254 +1
=========================================
+ Hits 17049 17066 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/calendars/ethiopian.py(1 hunks)holidays/countries/ethiopia.py(6 hunks)holidays/groups/christian.py(5 hunks)snapshots/countries/ET_COMMON.json(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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.
📚 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/calendars/ethiopian.pyholidays/groups/christian.pyholidays/countries/ethiopia.py
📚 Learning: 2025-04-03T05:59:57.480Z
Learnt from: PPsyrius
PR: vacanza/holidays#2407
File: snapshots/countries/TL_COMMON.json:7-7
Timestamp: 2025-04-03T05:59:57.480Z
Learning: In the holidays project, snapshot files (like snapshots/countries/TL_COMMON.json) are auto-generated when running `make snapshot` and should not be manually edited. Semicolons (;) in holiday entries are used as separators when multiple holidays occur on the same date.
Applied to files:
snapshots/countries/ET_COMMON.json
📚 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/groups/christian.pyholidays/countries/ethiopia.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/groups/christian.pyholidays/countries/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.py
📚 Learning: 2025-04-23T09:53:20.043Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:99-101
Timestamp: 2025-04-23T09:53:20.043Z
Learning: For Ethiopian holidays in the en_ET locale, translations must maintain the exact official wording from government sources such as Negarit Gazeta Proclamations, even when they contain redundancies like the word "Day" appearing twice (e.g., "Ethiopian National Unity Day (Ethiopian Nations and Nationalities) Day").
Applied to files:
holidays/countries/ethiopia.py
📚 Learning: 2025-04-23T09:51:03.951Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:95-101
Timestamp: 2025-04-23T09:51:03.951Z
Learning: Ethiopian holiday translations should follow the exact wording from official government sources (such as Negarit Gazeta Proclamations), even when they contain seemingly redundant wording like "Day" appearing twice.
Applied to files:
holidays/countries/ethiopia.py
📚 Learning: 2025-04-13T20:41:56.613Z
Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:266-284
Timestamp: 2025-04-13T20:41:56.613Z
Learning: The Islamic holiday dates in the holidays library should only include officially verified dates, not predicted ones, to maintain accuracy. This is especially important for holidays that depend on lunar observations like Eid al-Fitr and Eid al-Adha.
Applied to files:
holidays/countries/ethiopia.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/ethiopia.py
🧬 Code Graph Analysis (2)
holidays/groups/christian.py (1)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(18-32)
holidays/countries/ethiopia.py (2)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(18-32)holidays/groups/christian.py (4)
ChristianHolidays(24-514)_add_epiphany_day(305-326)_add_ethiopian_new_year(328-342)_add_finding_of_true_cross(344-357)
🔇 Additional comments (10)
snapshots/countries/ET_COMMON.json (1)
705-706: Snapshots: 2025 updates align with confirmed-year changes. LGTM.Eid al-Adha label un-estimated and Mawlid moved to Sep 5 match EthiopiaIslamicHolidays updates. Assuming this file was regenerated via make snapshot, this looks good.
holidays/groups/christian.py (3)
41-44: Christmas day extension to Ethiopian calendar: OKJan 7 for Julian or Ethiopian calendars is correct.
58-65: Ethiopian calendar detection helper: OKSimple and consistent with other calendar checks.
87-96: Calendar validation now includes Ethiopian calendar: OKError message lists all supported calendars. Looks good.
holidays/countries/ethiopia.py (6)
32-33: Reference URL refresh: OKtimeanddate link updated to 2025 snapshot. Fine.
51-51: Initialize ChristianHolidays with ETHIOPIAN_CALENDAR: OKThis correctly scopes all Christian computations to Ethiopian calendar.
62-64: Epiphany delegated to group helper: OK (after leap fix).Delegation is good; relies on corrected Ethiopian leap logic in ChristianHolidays.
Please re-run tests after applying the Epiphany leap fix in holidays/groups/christian.py.
86-90: Use Ethiopian-specific helpers for New Year and Meskel: OKCleaner and centralizes leap logic.
93-97: Popular Revolution Commemoration Day: Ethiopian leap logic: OKSwitch to is_ethiopian_leap_year removes Gregorian-century pitfalls. Good.
128-141: Islamic holidays: confirmed-year ranges updated to 2025; Mawlid 2025 added: OK
- EID_AL_FITR and EID_AL_ADHA confirmed through 2025 align with snapshot entries (no “estimated”).
- MAWLID 2025 set to Sep 5 matches snapshot.
KJhellico
reviewed
Aug 11, 2025
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/calendars/ethiopian.py(1 hunks)holidays/groups/christian.py(5 hunks)holidays/groups/international.py(2 hunks)tests/calendars/test_ethiopian.py(1 hunks)
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
📚 Learning: 2025-08-11T10:20:13.716Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Applied to files:
holidays/groups/international.pytests/calendars/test_ethiopian.pyholidays/calendars/ethiopian.pyholidays/groups/christian.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/groups/international.pytests/calendars/test_ethiopian.pyholidays/calendars/ethiopian.pyholidays/groups/christian.py
📚 Learning: 2025-08-11T10:14:28.499Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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/groups/international.pyholidays/calendars/ethiopian.pyholidays/groups/christian.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_ethiopian.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_ethiopian.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/calendars/test_ethiopian.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/calendars/test_ethiopian.py
📚 Learning: 2025-04-05T04:29:38.042Z
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.
Applied to files:
tests/calendars/test_ethiopian.py
📚 Learning: 2025-08-11T10:14:14.380Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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/groups/christian.py
📚 Learning: 2025-04-23T09:51:03.951Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:95-101
Timestamp: 2025-04-23T09:51:03.951Z
Learning: Ethiopian holiday translations should follow the exact wording from official government sources (such as Negarit Gazeta Proclamations), even when they contain seemingly redundant wording like "Day" appearing twice.
Applied to files:
holidays/groups/christian.py
📚 Learning: 2025-04-23T09:53:20.043Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:99-101
Timestamp: 2025-04-23T09:53:20.043Z
Learning: For Ethiopian holidays in the en_ET locale, translations must maintain the exact official wording from government sources such as Negarit Gazeta Proclamations, even when they contain redundancies like the word "Day" appearing twice (e.g., "Ethiopian National Unity Day (Ethiopian Nations and Nationalities) Day").
Applied to files:
holidays/groups/christian.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/groups/christian.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/groups/christian.py
📚 Learning: 2025-07-08T10:20:04.069Z
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:131-131
Timestamp: 2025-07-08T10:20:04.069Z
Learning: In Palestine, both the first and second days of Christmas (Orthodox and Catholic) are holidays. The first day is a nationwide public holiday, while the second day is a holiday for the relevant religious groups (Catholic or Orthodox) with the same name. The implementation correctly uses `_add_christmas_day` for nationwide holidays and `_add_christmas_day_two` for group-specific second days.
Applied to files:
holidays/groups/christian.py
📚 Learning: 2025-06-06T14:40:31.932Z
Learnt from: KJhellico
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:66-110
Timestamp: 2025-06-06T14:40:31.932Z
Learning: In the holidays library, within the _populate_public_holidays method, holidays should be arranged by calendar type (Islamic holidays first, then Gregorian holidays) without additional type grouping comments. The organization by calendar type is sufficient and follows the project's established conventions.
Applied to files:
holidays/groups/christian.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/groups/christian.py
📚 Learning: 2025-07-08T10:21:37.055Z
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:122-122
Timestamp: 2025-07-08T10:21:37.055Z
Learning: In Palestine, Easter follows the same naming pattern as Christmas - both the first and second days of Easter have identical names for the respective religious groups (Catholic or Orthodox). The implementation correctly uses `_add_easter_sunday` for nationwide holidays and `_add_easter_monday` for group-specific second days, but both share the same Arabic name "عيد الفصح المجيد".
Applied to files:
holidays/groups/christian.py
🧬 Code Graph Analysis (3)
holidays/groups/international.py (1)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-27)
tests/calendars/test_ethiopian.py (1)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-27)
holidays/groups/christian.py (1)
holidays/calendars/ethiopian.py (2)
is_ethiopian_leap_year(16-27)is_previous_year_ethiopian_leap_year(30-41)
⏰ 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 (9)
holidays/calendars/ethiopian.py (1)
16-27: Leap-year rule: correct and future-proof.Using the pure 4-year cycle via
(year + 1) % 4 == 0is correct for Ethiopian/Coptic and avoids Gregorian century exceptions. LGTM.holidays/groups/international.py (2)
15-15: Import is correct and minimal.Importing only
is_ethiopian_leap_yearkeeps dependencies tight. LGTM.
89-104: Ethiopian New Year helper: logic and doc are correct.Choosing Sep 12 in Ethiopian leap years and Sep 11 otherwise is correct. Docstring preserves source-accurate phrasing (“in coincidence of”). LGTM.
holidays/groups/christian.py (6)
17-21: Imports: correct symbols and scope.Brings in the needed Ethiopian calendar symbols without over-importing. LGTM.
43-47: Christmas day mapping: correct inclusion of Ethiopian calendar.Jan 7 for Julian and Ethiopian calendars is appropriate; Dec 25 otherwise. LGTM.
61-68: Ethiopian calendar detector: simple and clear.Static equality check keeps it consistent with other calendar checks. LGTM.
90-99: Calendar verification: accepts Ethiopian calendar and updates message.Validation and error message are consistent. LGTM.
320-330: Epiphany (Timkat): future-proof leap handling; add boundary tests.Using
is_previous_year_ethiopian_leap_year(self._year)correctly captures Tir 11 mapping and avoids Gregorian century exceptions (e.g., 2100). Suggest adding tests for 2099–2101 to lock behavior.Proposed test sketch (add to an appropriate test module, e.g., tests/countries/test_ethiopia.py):
def test_ethiopian_epiphany_dates(): et = Ethiopia() # or appropriate country class init # 2099: not previous-year Ethiopian leap -> Jan 19 # 2100: previous-year Ethiopian leap -> Jan 20 # 2101: not previous-year Ethiopian leap -> Jan 19 expected = { } for y, d in expected.items(): with self.subTest(year=y): self.assertHolidayName("Epiphany", et, d)
331-344: Finding of True Cross (Meskel): correct Ethiopian leap mapping and doc.Sep 28 in Ethiopian leap years, Sep 27 otherwise. Doc keeps “holiday” phrasing per source-accurate convention. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/calendars/test_ethiopian.py (2)
39-40: Use subTest for clearer failure granularity.Wrap each iteration to pinpoint failing years.
- for year in known_ethiopian_leap_years: - self.assertEqual(known_ethiopian_leap_years[year], is_ethiopian_leap_year(year)) + for year, expected in known_ethiopian_leap_years.items(): + with self.subTest(year=year): + self.assertEqual(expected, is_ethiopian_leap_year(year))
59-63: Mirror subTest pattern here as well.Same rationale for isolating failures by year.
- for year in known_previous_year_ethiopian_leap_years: - self.assertEqual( - known_previous_year_ethiopian_leap_years[year], - is_previous_year_ethiopian_leap_year(year), - ) + for year, expected in known_previous_year_ethiopian_leap_years.items(): + with self.subTest(year=year): + self.assertEqual( + expected, + is_previous_year_ethiopian_leap_year(year), + )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/calendars/test_ethiopian.py(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
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.
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-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:
tests/calendars/test_ethiopian.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_ethiopian.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_ethiopian.py
📚 Learning: 2025-08-11T10:20:13.716Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Applied to files:
tests/calendars/test_ethiopian.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/calendars/test_ethiopian.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_ethiopian.py
📚 Learning: 2025-04-05T04:29:38.042Z
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.
Applied to files:
tests/calendars/test_ethiopian.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/calendars/test_ethiopian.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_ethiopian.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/calendars/test_ethiopian.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/calendars/test_ethiopian.py
📚 Learning: 2025-06-18T10:10:46.158Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: tests/countries/test_france.py:24-27
Timestamp: 2025-06-18T10:10:46.158Z
Learning: The holidays library prioritizes full test coverage over performance in test suites. Modern library-wide standards favor comprehensive testing across all subdivisions and wide year ranges (e.g., 1803-2050 for France) to ensure thorough validation, even if it creates many test objects.
Applied to files:
tests/calendars/test_ethiopian.py
🧬 Code Graph Analysis (1)
tests/calendars/test_ethiopian.py (1)
holidays/calendars/ethiopian.py (2)
is_ethiopian_leap_year(16-27)is_previous_year_ethiopian_leap_year(30-41)
⏰ 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 (1)
tests/calendars/test_ethiopian.py (1)
42-58: Companion test for previous-year helper looks good.Covers the critical 2098–2201 window and aligns with the helper’s definition. Solid.
KJhellico
reviewed
Aug 11, 2025
KJhellico
reviewed
Aug 11, 2025
|
This PR now entails proper Julian Calendar Drift Adjustment for all Christian holidays as well 🎉 I did check just to be sure, and turns out |
KJhellico
reviewed
Aug 12, 2025
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
holidays/calendars/ethiopian.py(1 hunks)holidays/calendars/julian.py(1 hunks)holidays/countries/ethiopia.py(6 hunks)holidays/groups/christian.py(6 hunks)holidays/groups/international.py(2 hunks)tests/calendars/test_ethiopian.py(1 hunks)tests/calendars/test_julian.py(1 hunks)
🧰 Additional context used
🧠 Learnings (35)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
📚 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/groups/international.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.pyholidays/groups/christian.pyholidays/countries/ethiopia.py
📚 Learning: 2025-08-11T10:20:13.716Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Applied to files:
holidays/groups/international.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.pyholidays/groups/christian.pyholidays/countries/ethiopia.py
📚 Learning: 2025-08-11T10:14:28.499Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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/groups/international.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.pyholidays/groups/christian.pyholidays/countries/ethiopia.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_julian.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.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_julian.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.py
📚 Learning: 2025-06-18T10:10:46.158Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: tests/countries/test_france.py:24-27
Timestamp: 2025-06-18T10:10:46.158Z
Learning: The holidays library prioritizes full test coverage over performance in test suites. Modern library-wide standards favor comprehensive testing across all subdivisions and wide year ranges (e.g., 1803-2050 for France) to ensure thorough validation, even if it creates many test objects.
Applied to files:
tests/calendars/test_julian.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/calendars/test_julian.pytests/calendars/test_ethiopian.py
📚 Learning: 2025-08-11T13:48:45.905Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/calendars/ethiopian.py:13-13
Timestamp: 2025-08-11T13:48:45.905Z
Learning: The holidays library does not use `__all__` declarations in calendar modules (holidays/calendars/). Calendar files follow a standard pattern of defining constants and functions directly without explicit exports, similar to the convention used in country modules.
Applied to files:
holidays/calendars/ethiopian.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/calendars/ethiopian.py
📚 Learning: 2025-06-18T15:34:18.326Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/saint_pierre_and_miquelon.py:39-48
Timestamp: 2025-06-18T15:34:18.326Z
Learning: The holidays library does not use `__all__` declarations in country modules. Country files follow a standard pattern without explicit exports, and suggesting `__all__` goes against the established library-wide convention.
Applied to files:
holidays/calendars/ethiopian.py
📚 Learning: 2025-06-10T12:43:10.577Z
Learnt from: ankushhKapoor
PR: vacanza/holidays#2601
File: holidays/calendars/mongolian.py:1-1
Timestamp: 2025-06-10T12:43:10.577Z
Learning: The holidays project's calendar files (in holidays/calendars/) follow a consistent pattern of NOT having module-level docstrings. They only use class-level docstrings for their main classes. When reviewing calendar files, maintain this consistency and do not suggest adding module docstrings.
Applied to files:
holidays/calendars/ethiopian.py
📚 Learning: 2025-06-06T14:40:31.932Z
Learnt from: KJhellico
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:66-110
Timestamp: 2025-06-06T14:40:31.932Z
Learning: In the holidays library, within the _populate_public_holidays method, holidays should be arranged by calendar type (Islamic holidays first, then Gregorian holidays) without additional type grouping comments. The organization by calendar type is sufficient and follows the project's established conventions.
Applied to files:
holidays/calendars/ethiopian.pyholidays/groups/christian.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/calendars/ethiopian.pyholidays/groups/christian.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/calendars/ethiopian.pyholidays/groups/christian.py
📚 Learning: 2025-08-11T10:14:14.380Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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/ethiopian.pyholidays/groups/christian.pyholidays/countries/ethiopia.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/calendars/test_ethiopian.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_ethiopian.py
📚 Learning: 2025-06-16T15:48:48.680Z
Learnt from: PPsyrius
PR: vacanza/holidays#2615
File: tests/countries/test_anguilla.py:1-12
Timestamp: 2025-06-16T15:48:48.680Z
Learning: Test files in the holidays repository follow a standardized structure without module or class docstrings. All country test files use the same pattern: license header, imports, and class definition (`class Test{Country}(CommonCountryTests, TestCase):`) without docstrings. This is an established codebase convention that should be maintained for consistency.
Applied to files:
tests/calendars/test_ethiopian.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/calendars/test_ethiopian.py
📚 Learning: 2025-04-05T04:29:38.042Z
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.
Applied to files:
tests/calendars/test_ethiopian.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/calendars/test_ethiopian.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/calendars/test_ethiopian.py
📚 Learning: 2025-04-23T09:51:03.951Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:95-101
Timestamp: 2025-04-23T09:51:03.951Z
Learning: Ethiopian holiday translations should follow the exact wording from official government sources (such as Negarit Gazeta Proclamations), even when they contain seemingly redundant wording like "Day" appearing twice.
Applied to files:
holidays/groups/christian.pyholidays/countries/ethiopia.py
📚 Learning: 2025-04-23T09:53:20.043Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:99-101
Timestamp: 2025-04-23T09:53:20.043Z
Learning: For Ethiopian holidays in the en_ET locale, translations must maintain the exact official wording from government sources such as Negarit Gazeta Proclamations, even when they contain redundancies like the word "Day" appearing twice (e.g., "Ethiopian National Unity Day (Ethiopian Nations and Nationalities) Day").
Applied to files:
holidays/groups/christian.pyholidays/countries/ethiopia.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/groups/christian.py
📚 Learning: 2025-07-08T10:20:04.069Z
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:131-131
Timestamp: 2025-07-08T10:20:04.069Z
Learning: In Palestine, both the first and second days of Christmas (Orthodox and Catholic) are holidays. The first day is a nationwide public holiday, while the second day is a holiday for the relevant religious groups (Catholic or Orthodox) with the same name. The implementation correctly uses `_add_christmas_day` for nationwide holidays and `_add_christmas_day_two` for group-specific second days.
Applied to files:
holidays/groups/christian.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.py
📚 Learning: 2025-04-13T20:41:56.613Z
Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:266-284
Timestamp: 2025-04-13T20:41:56.613Z
Learning: The Islamic holiday dates in the holidays library should only include officially verified dates, not predicted ones, to maintain accuracy. This is especially important for holidays that depend on lunar observations like Eid al-Fitr and Eid al-Adha.
Applied to files:
holidays/countries/ethiopia.py
🧬 Code Graph Analysis (5)
holidays/groups/international.py (4)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-35)holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)holidays/holiday_base.py (1)
_add_holiday(797-809)
tests/calendars/test_julian.py (1)
holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)
tests/calendars/test_ethiopian.py (1)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-35)
holidays/groups/christian.py (4)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-35)holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)holidays/holiday_base.py (1)
_add_holiday(797-809)
holidays/countries/ethiopia.py (6)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-35)holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)holidays/groups/christian.py (3)
ChristianHolidays(23-503)_add_epiphany_day(306-330)_add_finding_of_true_cross(332-346)holidays/groups/international.py (1)
_add_ethiopian_new_year(90-106)holidays/holiday_base.py (1)
_add_holiday(797-809)
🔇 Additional comments (19)
holidays/calendars/julian.py (1)
27-27: Formula looks correct and matches historical drift.The piecewise return (−13 for ≤1582; otherwise c−q−15) yields expected values (e.g., 1900:0, 2000:0, 2100:+1). Tests cover representative points.
holidays/groups/international.py (1)
15-17: Imports are appropriate and cohesive.All four imports are required by the new helper; consistent with project patterns.
holidays/calendars/ethiopian.py (2)
13-13: Constant definition is consistent with calendar module patterns.Name and placement align with other calendar modules.
16-35: Ethiopian leap-year helper is correct and future-proof.
- Pure 4-year rule without Gregorian century exceptions: correct.
- is_previous param addresses “ending in year Y” use-cases cleanly.
tests/calendars/test_ethiopian.py (1)
18-60: Good coverage for both current and previous-year semantics (incl. 2100+).Mappings correctly reflect ((y+1) % 4 == 0) and (y % 4 == 0). This locks behavior beyond century years.
holidays/groups/christian.py (7)
17-19: LGTM! Clean imports for Ethiopian calendar support.The addition of Ethiopian calendar imports alongside the existing Julian calendar infrastructure sets up proper support for Ethiopian-specific date calculations.
39-43: Good use of drift adjustment for Ethiopian calendar.Correctly applies Julian calendar drift to Ethiopian calendar dates since both follow the same Julian leap year rules. The Ethiopian Orthodox Church uses January 7 (Julian) for Christmas.
57-64: Clean static method addition.Well-structured helper following the existing pattern for calendar detection methods.
175-180: Correct drift application for Ethiopian Assumption of Mary.The Ethiopian Orthodox Church observes this feast on August 28 (Julian calendar), which requires drift adjustment.
318-331: Epiphany implementation correctly handles Ethiopian leap years.The use of
is_previous=Trueparameter is correct here since Epiphany falls in January, before the Ethiopian New Year transition in September. This aligns with the established pattern from previous reviews.
332-346: Finding of True Cross implementation looks solid.Meskel dates (Sep 27/28) correctly use Ethiopian leap year check since they fall after the Ethiopian New Year transition. The drift adjustment is properly applied.
86-95: Validation includes Ethiopian calendar
TheETHIOPIAN_CALENDARconstant is defined inholidays/calendars/ethiopian.pyand the snippet inholidays/groups/christian.pyproperly references it in the validation set. The ValueError covers all supported calendars—no further changes needed.holidays/countries/ethiopia.py (7)
13-19: Clean migration to Ethiopian calendar imports.Properly replaces Julian calendar with Ethiopian-specific imports while keeping necessary utilities.
53-53: Correct calendar initialization.Using
ETHIOPIAN_CALENDARinstead ofJULIAN_CALENDARproperly aligns the holiday calculations with Ethiopian Orthodox traditions.
65-65: Simplified Epiphany implementation.Good use of the centralized helper method instead of duplicating date logic.
88-88: Clean use of Ethiopian New Year helper.Leverages the shared implementation from parent class.
91-91: Finding of True Cross properly delegated.Uses the parent class helper for Meskel calculation.
94-101: Popular Revolution Day calculation handles leap years correctly.The date shifts from Sep 12 to Sep 13 in Ethiopian leap years, with proper drift adjustment applied.
133-144: Islamic holiday coverage extended to 2025.Good to see the confirmed years and dates updated with official 2025 data.
KJhellico
reviewed
Aug 12, 2025
ETHIOPIAN_CALENDAR support for ChristianHolidaysETHIOPIAN_CALENDAR support, Julian Date Shift adjustment pre-1899 and post-2099
KJhellico
reviewed
Aug 12, 2025
ETHIOPIAN_CALENDAR support, Julian Date Shift adjustment pre-1899 and post-2099ETHIOPIAN_CALENDAR support, Julian Date Drift adjustment pre-1899 and post-2099
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
holidays/calendars/julian.py (1)
17-25: Docstring baseline off by one; tighten wording.The code returns 0 for 1900–2099, so the baseline in the docstring should be 1900–2099 (not 1899–2099). Also tweak wording for clarity.
Apply this diff:
-"""Return the Julian–Gregorian date drift relative to the 1899–2099 baseline. +"""Return the Julian–Gregorian date drift relative to the 1900–2099 baseline (0 in 1900–2099). @@ - year: - Gregorian year to check. + year: + Gregorian year to evaluate. @@ - Number of days to add/subtract relative to the 1899–2099 baseline. + Number of days to add/subtract relative to the 1900–2099 baseline. """tests/calendars/test_ethiopian.py (1)
36-37: Use subTest to isolate failures per year.This mirrors patterns in other calendar tests and improves failure diagnostics.
Apply this diff:
- for year in known_ethiopian_leap_years: - self.assertEqual(known_ethiopian_leap_years[year], is_ethiopian_leap_year(year)) + for year, expected in known_ethiopian_leap_years.items(): + with self.subTest(year=year): + self.assertEqual(expected, is_ethiopian_leap_year(year))holidays/groups/christian.py (1)
318-331: Implementation correctly handles Ethiopian Epiphany dates.The logic properly uses:
- Julian calendar: drift-adjusted January 19
- Ethiopian calendar: January 20 for Ethiopian leap years (checking
self._year - 1), otherwise January 19, both drift-adjusted- Other calendars: standard January 6
This aligns with the learning that Ethiopian Epiphany follows Gregorian leap year rules since it occurs before the Ethiopian calendar year transition in September.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
holidays/calendars/ethiopian.py(1 hunks)holidays/calendars/julian.py(1 hunks)holidays/groups/christian.py(6 hunks)holidays/groups/international.py(2 hunks)tests/calendars/test_ethiopian.py(1 hunks)tests/calendars/test_julian.py(1 hunks)
🧰 Additional context used
🧠 Learnings (28)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
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.
📚 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_julian.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.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_julian.pyholidays/calendars/ethiopian.pytests/calendars/test_ethiopian.py
📚 Learning: 2025-06-18T10:10:46.158Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: tests/countries/test_france.py:24-27
Timestamp: 2025-06-18T10:10:46.158Z
Learning: The holidays library prioritizes full test coverage over performance in test suites. Modern library-wide standards favor comprehensive testing across all subdivisions and wide year ranges (e.g., 1803-2050 for France) to ensure thorough validation, even if it creates many test objects.
Applied to files:
tests/calendars/test_julian.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/calendars/test_julian.pytests/calendars/test_ethiopian.py
📚 Learning: 2025-04-05T04:29:38.042Z
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.
Applied to files:
tests/calendars/test_julian.pytests/calendars/test_ethiopian.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/calendars/ethiopian.pytests/calendars/test_ethiopian.pyholidays/groups/international.pyholidays/groups/christian.py
📚 Learning: 2025-08-11T10:20:13.716Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Applied to files:
holidays/calendars/ethiopian.pytests/calendars/test_ethiopian.pyholidays/calendars/julian.pyholidays/groups/international.pyholidays/groups/christian.py
📚 Learning: 2025-08-11T10:14:28.499Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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/ethiopian.pytests/calendars/test_ethiopian.pyholidays/groups/international.pyholidays/groups/christian.py
📚 Learning: 2025-08-11T13:48:45.905Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/calendars/ethiopian.py:13-13
Timestamp: 2025-08-11T13:48:45.905Z
Learning: The holidays library does not use `__all__` declarations in calendar modules (holidays/calendars/). Calendar files follow a standard pattern of defining constants and functions directly without explicit exports, similar to the convention used in country modules.
Applied to files:
holidays/calendars/ethiopian.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/calendars/ethiopian.py
📚 Learning: 2025-06-18T15:34:18.326Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/saint_pierre_and_miquelon.py:39-48
Timestamp: 2025-06-18T15:34:18.326Z
Learning: The holidays library does not use `__all__` declarations in country modules. Country files follow a standard pattern without explicit exports, and suggesting `__all__` goes against the established library-wide convention.
Applied to files:
holidays/calendars/ethiopian.py
📚 Learning: 2025-06-10T12:43:10.577Z
Learnt from: ankushhKapoor
PR: vacanza/holidays#2601
File: holidays/calendars/mongolian.py:1-1
Timestamp: 2025-06-10T12:43:10.577Z
Learning: The holidays project's calendar files (in holidays/calendars/) follow a consistent pattern of NOT having module-level docstrings. They only use class-level docstrings for their main classes. When reviewing calendar files, maintain this consistency and do not suggest adding module docstrings.
Applied to files:
holidays/calendars/ethiopian.py
📚 Learning: 2025-06-06T14:40:31.932Z
Learnt from: KJhellico
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:66-110
Timestamp: 2025-06-06T14:40:31.932Z
Learning: In the holidays library, within the _populate_public_holidays method, holidays should be arranged by calendar type (Islamic holidays first, then Gregorian holidays) without additional type grouping comments. The organization by calendar type is sufficient and follows the project's established conventions.
Applied to files:
holidays/calendars/ethiopian.pyholidays/groups/christian.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/calendars/ethiopian.pyholidays/groups/christian.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/calendars/ethiopian.pyholidays/groups/christian.py
📚 Learning: 2025-08-11T10:14:14.380Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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/ethiopian.pyholidays/groups/international.pyholidays/groups/christian.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/calendars/test_ethiopian.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_ethiopian.py
📚 Learning: 2025-06-16T15:48:48.680Z
Learnt from: PPsyrius
PR: vacanza/holidays#2615
File: tests/countries/test_anguilla.py:1-12
Timestamp: 2025-06-16T15:48:48.680Z
Learning: Test files in the holidays repository follow a standardized structure without module or class docstrings. All country test files use the same pattern: license header, imports, and class definition (`class Test{Country}(CommonCountryTests, TestCase):`) without docstrings. This is an established codebase convention that should be maintained for consistency.
Applied to files:
tests/calendars/test_ethiopian.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/calendars/test_ethiopian.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/calendars/test_ethiopian.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/calendars/test_ethiopian.py
📚 Learning: 2025-07-08T10:20:04.069Z
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:131-131
Timestamp: 2025-07-08T10:20:04.069Z
Learning: In Palestine, both the first and second days of Christmas (Orthodox and Catholic) are holidays. The first day is a nationwide public holiday, while the second day is a holiday for the relevant religious groups (Catholic or Orthodox) with the same name. The implementation correctly uses `_add_christmas_day` for nationwide holidays and `_add_christmas_day_two` for group-specific second days.
Applied to files:
holidays/calendars/julian.pyholidays/groups/christian.py
📚 Learning: 2025-08-12T15:52:40.597Z
Learnt from: KJhellico
PR: vacanza/holidays#2794
File: holidays/groups/international.py:90-107
Timestamp: 2025-08-12T15:52:40.597Z
Learning: Ethiopian New Year (Enkutatash) falls on September 12 when the Ethiopian year that just ended was a leap year, not when the current Ethiopian year is a leap year. This is because Ethiopian New Year marks the transition between Ethiopian years, and the date depends on the leap year status of the concluding Ethiopian year.
Applied to files:
holidays/groups/international.py
📚 Learning: 2025-04-23T09:51:03.951Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:95-101
Timestamp: 2025-04-23T09:51:03.951Z
Learning: Ethiopian holiday translations should follow the exact wording from official government sources (such as Negarit Gazeta Proclamations), even when they contain seemingly redundant wording like "Day" appearing twice.
Applied to files:
holidays/groups/christian.py
📚 Learning: 2025-04-23T09:53:20.043Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:99-101
Timestamp: 2025-04-23T09:53:20.043Z
Learning: For Ethiopian holidays in the en_ET locale, translations must maintain the exact official wording from government sources such as Negarit Gazeta Proclamations, even when they contain redundancies like the word "Day" appearing twice (e.g., "Ethiopian National Unity Day (Ethiopian Nations and Nationalities) Day").
Applied to files:
holidays/groups/christian.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/groups/christian.py
🧬 Code Graph Analysis (4)
tests/calendars/test_julian.py (1)
holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)
tests/calendars/test_ethiopian.py (1)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-31)
holidays/groups/international.py (4)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-31)holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)holidays/holiday_base.py (1)
_add_holiday(797-809)
holidays/groups/christian.py (4)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-31)holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)holidays/holiday_base.py (1)
_add_holiday(797-809)
⏰ 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 build on windows-latest
🔇 Additional comments (8)
holidays/calendars/julian.py (1)
27-27: Logic matches historical drift; good.The piecewise formula yields expected values across historical ranges and aligns with tests (e.g., 1583:-3, 1700:-2, 1800:-1, 1900–2099:0, 2100:+1).
holidays/calendars/ethiopian.py (1)
31-31: Leap rule implementation is correct and future-proof.Using (year + 1) % 4 == 0 avoids Gregorian century exceptions and matches Coptic/Ethiopian rules.
holidays/groups/christian.py (6)
17-19: LGTM!The imports are properly organized, bringing in the Ethiopian calendar support and Julian drift calculation as needed for the new functionality.
57-64: LGTM!The static method for Ethiopian calendar detection follows the established pattern.
86-95: LGTM!The calendar verification is properly updated to include Ethiopian calendar support with clear error messaging.
332-346: LGTM!The Finding of True Cross implementation correctly calculates Meskel dates using Ethiopian leap year logic and applies the appropriate drift adjustment for September dates.
40-41: Christmas Day Julian Drift Adjustment ConfirmedThe use of
julian_calendar_drift(self._year - 1)for January 7 (Orthodox Christmas) aligns with how we handle Epiphany on January 19/20 elsewhere—no change required.• holidays/groups/christian.py, lines 40-41:
_timedelta(date(self._year, JAN, 7), julian_calendar_drift(self._year - 1))
• holidays/groups/christian.py, Epiphany logic around lines 322–326: same drift calculation
175-177: Ignore the January “–1” adjustment suggestion.I checked all uses of julian_calendar_drift in christian.py (Assumption of Mary, Christmas, etc.) and they uniformly apply
julian_calendar_drift(self._year)with no-1tweak for January. There’s no special offset logic anywhere in that file—so this review comment can be dismissed.Likely an incorrect or invalid review comment.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/groups/christian.py(6 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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.
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.
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).
📚 Learning: 2025-08-11T10:20:13.716Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Applied to files:
holidays/groups/christian.py
📚 Learning: 2025-08-11T10:14:28.499Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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/groups/christian.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/groups/christian.py
📚 Learning: 2025-08-11T10:14:14.380Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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/groups/christian.py
📚 Learning: 2025-04-23T09:51:03.951Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:95-101
Timestamp: 2025-04-23T09:51:03.951Z
Learning: Ethiopian holiday translations should follow the exact wording from official government sources (such as Negarit Gazeta Proclamations), even when they contain seemingly redundant wording like "Day" appearing twice.
Applied to files:
holidays/groups/christian.py
📚 Learning: 2025-04-23T09:53:20.043Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:99-101
Timestamp: 2025-04-23T09:53:20.043Z
Learning: For Ethiopian holidays in the en_ET locale, translations must maintain the exact official wording from government sources such as Negarit Gazeta Proclamations, even when they contain redundancies like the word "Day" appearing twice (e.g., "Ethiopian National Unity Day (Ethiopian Nations and Nationalities) Day").
Applied to files:
holidays/groups/christian.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/groups/christian.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/groups/christian.py
📚 Learning: 2025-07-08T10:20:04.069Z
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:131-131
Timestamp: 2025-07-08T10:20:04.069Z
Learning: In Palestine, both the first and second days of Christmas (Orthodox and Catholic) are holidays. The first day is a nationwide public holiday, while the second day is a holiday for the relevant religious groups (Catholic or Orthodox) with the same name. The implementation correctly uses `_add_christmas_day` for nationwide holidays and `_add_christmas_day_two` for group-specific second days.
Applied to files:
holidays/groups/christian.py
📚 Learning: 2025-06-06T14:40:31.932Z
Learnt from: KJhellico
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:66-110
Timestamp: 2025-06-06T14:40:31.932Z
Learning: In the holidays library, within the _populate_public_holidays method, holidays should be arranged by calendar type (Islamic holidays first, then Gregorian holidays) without additional type grouping comments. The organization by calendar type is sufficient and follows the project's established conventions.
Applied to files:
holidays/groups/christian.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/groups/christian.py
🧬 Code Graph Analysis (1)
holidays/groups/christian.py (4)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-31)holidays/calendars/gregorian.py (1)
_timedelta(37-42)holidays/calendars/julian.py (1)
julian_calendar_drift(16-27)holidays/holiday_base.py (1)
_add_holiday(797-809)
🔇 Additional comments (5)
holidays/groups/christian.py (5)
17-19: Imports for Ethiopian calendar and Julian drift look correct.The added imports are used below and align with the new drift-aware logic.
39-42: Christmas: Correct use of previous-year drift for January observance.Using
julian_calendar_drift(self._year - 1)for Jan 7 is the right choice for pre-1899/post-2099 handling.
57-64: Private Ethiopian calendar detector: LGTM.Consistent with existing private helpers and used correctly below.
86-95: Calendar validation extended to Ethiopian: LGTM.Error message and allowed set both updated coherently.
175-177: Assumption (Julian) drift application is correct.Using Aug 28 +
julian_calendar_drift(self._year)for Julian observance mirrors the intended pre/post-2099 logic.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/ethiopia.py(6 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/international.py:0-0
Timestamp: 2025-08-12T17:39:49.784Z
Learning: Ethiopian New Year (Enkutatash) calculation correctly uses `is_ethiopian_leap_year(self._year)` instead of `isleap(self._year + 1)` to properly handle century edge cases. The Ethiopian function `(year + 1) % 4 == 0` checks if the next year is divisible by 4 using Julian/Ethiopian rules (no century exceptions), while `isleap()` applies Gregorian century rules. This difference is crucial around years like 2100 where the two calendar systems diverge.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/international.py:0-0
Timestamp: 2025-08-12T17:39:49.784Z
Learning: Ethiopian New Year (Enkutatash) calculation uses `is_ethiopian_leap_year(self._year)` instead of `isleap(self._year + 1)` to properly handle century edge cases. Ethiopian leap years follow Julian rules (every 4 years without exception) while Gregorian leap years have century exceptions, causing different results around years like 2100.
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.
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).
📚 Learning: 2025-08-11T10:20:13.716Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:317-326
Timestamp: 2025-08-11T10:20:13.716Z
Learning: In the Ethiopian calendar implementation for Epiphany in `holidays/groups/christian.py`, the use of `isleap(self._year)` (Gregorian leap year check) is correct, not `is_ethiopian_leap_year(self._year)`. Ethiopian Epiphany (Timkat) falls on January 19 in ordinary years and January 20 in Gregorian leap years. This differs from other Ethiopian holidays like Ethiopian New Year and Finding of True Cross which occur after September and require the shifted Ethiopian leap year check.
Applied to files:
holidays/countries/ethiopia.py
📚 Learning: 2025-08-12T17:39:49.784Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/international.py:0-0
Timestamp: 2025-08-12T17:39:49.784Z
Learning: Ethiopian New Year (Enkutatash) calculation uses `is_ethiopian_leap_year(self._year)` instead of `isleap(self._year + 1)` to properly handle century edge cases. Ethiopian leap years follow Julian rules (every 4 years without exception) while Gregorian leap years have century exceptions, causing different results around years like 2100.
Applied to files:
holidays/countries/ethiopia.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/ethiopia.py
📚 Learning: 2025-08-12T17:39:49.784Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/international.py:0-0
Timestamp: 2025-08-12T17:39:49.784Z
Learning: Ethiopian New Year (Enkutatash) calculation correctly uses `is_ethiopian_leap_year(self._year)` instead of `isleap(self._year + 1)` to properly handle century edge cases. The Ethiopian function `(year + 1) % 4 == 0` checks if the next year is divisible by 4 using Julian/Ethiopian rules (no century exceptions), while `isleap()` applies Gregorian century rules. This difference is crucial around years like 2100 where the two calendar systems diverge.
Applied to files:
holidays/countries/ethiopia.py
📚 Learning: 2025-08-11T10:14:28.499Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.499Z
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/countries/ethiopia.py
📚 Learning: 2025-08-11T10:14:14.380Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: holidays/groups/christian.py:344-357
Timestamp: 2025-08-11T10:14:14.380Z
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/countries/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.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/ethiopia.py
📚 Learning: 2025-04-23T09:53:20.043Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:99-101
Timestamp: 2025-04-23T09:53:20.043Z
Learning: For Ethiopian holidays in the en_ET locale, translations must maintain the exact official wording from government sources such as Negarit Gazeta Proclamations, even when they contain redundancies like the word "Day" appearing twice (e.g., "Ethiopian National Unity Day (Ethiopian Nations and Nationalities) Day").
Applied to files:
holidays/countries/ethiopia.py
📚 Learning: 2025-04-23T09:51:03.951Z
Learnt from: PPsyrius
PR: vacanza/holidays#2490
File: holidays/locale/en_ET/LC_MESSAGES/ET.po:95-101
Timestamp: 2025-04-23T09:51:03.951Z
Learning: Ethiopian holiday translations should follow the exact wording from official government sources (such as Negarit Gazeta Proclamations), even when they contain seemingly redundant wording like "Day" appearing twice.
Applied to files:
holidays/countries/ethiopia.py
📚 Learning: 2025-04-13T20:41:56.613Z
Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:266-284
Timestamp: 2025-04-13T20:41:56.613Z
Learning: The Islamic holiday dates in the holidays library should only include officially verified dates, not predicted ones, to maintain accuracy. This is especially important for holidays that depend on lunar observations like Eid al-Fitr and Eid al-Adha.
Applied to files:
holidays/countries/ethiopia.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/ethiopia.py
🧬 Code Graph Analysis (1)
holidays/countries/ethiopia.py (4)
holidays/calendars/ethiopian.py (1)
is_ethiopian_leap_year(16-31)holidays/groups/christian.py (2)
_add_epiphany_day(306-328)_add_finding_of_true_cross(330-342)holidays/groups/international.py (1)
_add_ethiopian_new_year(90-102)holidays/holiday_base.py (1)
_add_holiday(797-809)
⏰ 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 (8)
holidays/countries/ethiopia.py (8)
16-17: Ethiopian calendar imports consolidated — OKGood move pulling ETHIOPIAN_CALENDAR and is_ethiopian_leap_year from the dedicated module and trimming unused Julian/Gregorian symbols. Keeps responsibilities clear and avoids accidental Gregorian assumptions.
32-32: Reference URL refreshed — OKArchiving to a 2025 snapshot is appropriate for traceability.
51-51: Initialize ChristianHolidays with ETHIOPIAN_CALENDAR — correct and future-proofThis aligns the Christian helpers with Ethiopian/Julian behavior system-wide, enabling reuse of drift-aware helpers and avoiding per-country bespoke logic.
86-86: Ethiopian New Year via helper — correct leap handlingUsing
_add_ethiopian_new_year()(drift-aware) withis_ethiopian_leap_year(self._year)matches Ethiopian rules and avoids Gregorian century pitfalls (e.g., 2100).
89-89: Finding of True Cross via helper — correct drift + Ethiopian leap shiftSwitching to
_add_finding_of_true_cross()centralizes Meskel logic and correctly applies drift and Ethiopian leap-day adjustment.
129-129: Extend Eid al-Adha confirmed years to 2025 — OKMatches the stated objective of updating Islamic dates to 2025. Local overrides remain minimal (only 2018), which is consistent with project patterns for country-specific deviations.
137-137: Extend Mawlid confirmed years to 2025 — OKConsistent with policy to include only officially verified years. Good.
141-141: Add 2025 Mawlid date (Sep 5) — OKDate aligns with known 2025 observance. No docstrings added to the custom class, as per convention.
KJhellico
reviewed
Aug 12, 2025
|
I have been thinking about why Ethiopian Christmas is always listed on January 7, even though it should follow the same rule as Epiphany (and other religious holidays). And I found out that it should really be Tahsas 29, which can also be January 8. |
|
It's a bit strange, but if it's really so, then ok. 🤷♂️ |
arkid15r
reviewed
Aug 14, 2025
There was a problem hiding this comment.
It's a great improvement and quite well-structured refactoring. Thank you @PPsyrius (and @KJhellico for the review 👍 )
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
ETHIOPIAN_CALENDARsupport toChristianHolidaysso that they can be used by Add Eritrea holidays #2783 as well.Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.