Add Benin holidays #2562
Add Benin holidays #2562
Conversation
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update adds support for Benin as a new country in the holidays library. It includes a holiday class, registry entry, localization files in English and French, and corresponding tests. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 300000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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 (7)
README.md(2 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/benin.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/BJ.po(1 hunks)holidays/locale/fr_BJ/LC_MESSAGES/BJ.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_benin.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (1)
holidays/countries/benin.py (3)
Benin(20-131)BJ(134-135)BEN(138-139)
🔇 Additional comments (19)
holidays/registry.py (1)
43-43: Benin added to registry
Looks good—Benin maps to ("Benin", "BJ", "BEN") and aligns with other entries.README.md (2)
108-108: Updated supported country count
Great catch on bumping the total to 175 to include Benin.
290-295: Inserted Benin in countries table
The new Benin row follows the existing table format with correct codes and languages.holidays/countries/__init__.py (1)
35-35: Imported Benin in namespace
Thefrom .benin import Benin, BJ, BENline is correctly placed alphabetically between Belize and Bolivia.holidays/locale/fr_BJ/LC_MESSAGES/BJ.po (2)
17-27: PO header metadata is correct
Headers like Project-Id-Version and Language are set properly for fr_BJ locale.
29-95: French translations skeleton in place
All holiday msgid entries are present; empty msgstr placeholders will fallback to msgid for French defaults.holidays/locale/en_US/LC_MESSAGES/BJ.po (2)
17-27: PO header metadata is correct
Header fields accurately reflect en_US locale and source fr_BJ.
29-95: English translations are accurate
All French msgid values have proper English msgstr mappings, covering annotations and holiday names.tests/countries/test_benin.py (4)
22-24: Good test setup with comprehensive year coverage.The test setup properly covers the full range from 1960-2050 and correctly configures the non-observed years parameter. The additional
no_estimated_holidaysinstance is well thought out for testing Islamic holidays without estimation labels.
45-50: Well-implemented observed holiday testing.The test correctly verifies that Vodoun Festival in 2024 has both the regular holiday on Jan 10 and the observed holiday on Jan 9. This matches the special handling in the implementation.
155-196: Comprehensive Islamic holiday testing.The tests properly use the
no_estimated_holidaysinstance to verify Islamic holidays without estimation labels, and the regular instance would show the estimated labels. The date ranges and specific dates look accurate.
197-230: Excellent localization coverage.Both default French and English (US) localizations are thoroughly tested, including proper handling of estimated Islamic holiday labels in both languages.
holidays/countries/benin.py (7)
20-66: Excellent class structure and configuration.The class properly inherits from all necessary base classes and has well-configured attributes. The localization labels for observed and estimated holidays are properly implemented with gettext translation support.
53-53: Verify 2025 Vodoun Festival reference accuracy.The reference mentions "9 janvier 2024 et vendredi 10 janvier 2024" but this is in the 2025 section. This seems to be referencing 2024 dates rather than 2025.
Please verify if this reference is accurate for 2025 or if it should reference the correct 2025 dates.
67-77: Clean constructor implementation.The constructor properly handles the Islamic estimated holidays parameter and correctly initializes all parent classes in the right order.
83-90: Smart handling of Vodoun Festival observed day.The special case for 2024 where Vodoun Festival (Jan 10) was observed on Jan 9 is well implemented. This shows attention to real-world holiday observance patterns.
100-109: Proper handling of Whit Monday exception.The custom date handling for Whit Monday in 2023 (May 29 instead of the calculated date) is cleanly implemented with a clear data structure approach.
122-132: Complete Islamic holiday coverage.All three major Islamic holidays (Prophet's Birthday, Eid al-Fitr, Eid al-Adha) are properly implemented with their local names (Maouloud, Korité, Tabaski).
134-139: Proper country alias implementation.The BJ and BEN alias classes follow the library's standard pattern for providing alternative country codes.
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
|
Please merge latest |
|
It would also be good to add holidays from Art. 3 of Law 90-019, in WORKDAY category. |
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/countries/test_benin.py (1)
32-35: Consider using consistent generator patterns for fixed date holidays.Following the project's pattern for fixed date holidays, these could be simplified using generators as shown in past comments.
For example:
- self.assertHolidayName( - "Fête du Nouvel An", (f"{year}-01-01" for year in range(1960, 2050)) - ) + self.assertHolidayName("Fête du Nouvel An", (f"{year}-01-01" for year in range(1960, 2050)))Also applies to: 68-69, 98-100, 127-128
holidays/countries/benin.py (1)
24-29: Apply Internet Archive treatment to legal reference links.Based on past review feedback, the legal reference links should be archived for long-term accessibility, similar to how the other reference links in this file are handled.
- * [Order 71-14](https://sgg.gouv.bj/doc/ordonnance-1971-14/) - * [Order 76-35](https://sgg.gouv.bj/doc/ordonnance-1976-35/) - * [Order 79-33](https://sgg.gouv.bj/doc/ordonnance-1979-33/) - * [Law 90-019](https://sgg.gouv.bj/doc/loi-90-019/) - * [Law 97-031](https://sgg.gouv.bj/doc/loi-97-031/) - * [Law 2024-32](https://sgg.gouv.bj/doc/loi-2024-32/) + * [Order 71-14](https://web.archive.org/web/20241210000000/https://sgg.gouv.bj/doc/ordonnance-1971-14/) + * [Order 76-35](https://web.archive.org/web/20241210000001/https://sgg.gouv.bj/doc/ordonnance-1976-35/) + * [Order 79-33](https://web.archive.org/web/20241210000002/https://sgg.gouv.bj/doc/ordonnance-1979-33/) + * [Law 90-019](https://web.archive.org/web/20241210000003/https://sgg.gouv.bj/doc/loi-90-019/) + * [Law 97-031](https://web.archive.org/web/20241210000004/https://sgg.gouv.bj/doc/loi-97-031/) + * [Law 2024-32](https://web.archive.org/web/20241210000005/https://sgg.gouv.bj/doc/loi-2024-32/)Note: Use actual Internet Archive URLs when available.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)holidays/countries/benin.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/BJ.po(1 hunks)holidays/locale/fr_BJ/LC_MESSAGES/BJ.po(1 hunks)tests/countries/test_benin.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_benin.py (1)
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.
🪛 Ruff (0.11.9)
holidays/countries/benin.py
16-16: holidays.observed_holiday_base.FRI_TO_PREV_THU imported but unused
Remove unused import: holidays.observed_holiday_base.FRI_TO_PREV_THU
(F401)
🔇 Additional comments (11)
README.md (1)
108-108: Changes look good!The country count update and new Benin table entry are properly formatted and correctly positioned alphabetically. The supported languages align with the localization files included in this PR.
Also applies to: 290-295
holidays/locale/en_US/LC_MESSAGES/BJ.po (1)
1-111: English translations are well done!The localization file follows proper gettext format and provides accurate English translations for all Benin holiday names and observance annotations. The metadata is complete and the translation quality is appropriate.
holidays/locale/fr_BJ/LC_MESSAGES/BJ.po (1)
1-111: Source language file structure is correct!This French (Benin) localization file properly serves as the source language file with empty msgstr values, which is the correct format when the msgid values are already in the target language.
tests/countries/test_benin.py (1)
19-213: Comprehensive test coverage looks great!The test suite provides excellent coverage of all Benin holidays with proper date ranges, localization testing, and handling of estimated Islamic holidays. The structure follows project conventions well.
holidays/countries/benin.py (7)
61-67: Implementation looks solid.The country code, language settings, and start year configuration align well with the legal references and follow the library's standard patterns.
69-79: Constructor properly handles Islamic holiday estimation.The implementation correctly follows the library pattern for Islamic holidays with proper parameter handling and initialization order.
85-93: Vodoun Festival implementation handles 2025 law change correctly.The logic properly implements the transition from a single-day holiday (January 10) to a two-day holiday (second Friday of January and the day before) starting in 2025, as specified in Law 2024-32.
101-107: Easter holiday transition logic is accurate.The implementation correctly switches from Easter Sunday (pre-1991) to Easter Monday (1991+) based on the legal framework changes, following Law 90-019.
134-140: National Day date transition properly implemented.The logic correctly handles the change from November 30 (pre-1991) to August 1 (1991+) for National Day, following the historical legal changes.
150-158: Islamic holidays implementation follows legal framework.The Prophet's Birthday addition from 1990 onwards and the consistent inclusion of Eid al-Fitr and Eid al-Adha align with Law 90-019 and subsequent legal provisions.
161-166: Country code aliases follow standard pattern.The BJ and BEN alias classes are implemented correctly following the library's conventions for country code variations.
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
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 (5)
README.md(2 hunks)holidays/countries/benin.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/BJ.po(1 hunks)holidays/locale/fr_BJ/LC_MESSAGES/BJ.po(1 hunks)tests/countries/test_benin.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_benin.py (1)
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.
🧬 Code Graph Analysis (1)
tests/countries/test_benin.py (2)
tests/common.py (7)
TestCase(28-338)CommonCountryTests(356-374)assertAliases(121-130)assertNoHolidays(292-294)assertHolidayName(195-199)assertNoHolidayName(273-275)assertLocalizedHolidays(327-338)holidays/countries/benin.py (3)
Benin(20-171)BJ(174-175)BEN(178-179)
🔇 Additional comments (21)
README.md (2)
108-108: Country count update looks good.The increment from 177 to 178 countries correctly reflects the addition of Benin.
290-295: Benin entry is properly formatted and positioned.The table entry follows the established pattern with correct country code, language specifications, and category information. The alphabetical placement between Belize and Bolivia is appropriate.
holidays/locale/fr_BJ/LC_MESSAGES/BJ.po (2)
17-17: Project version is appropriate.The version "Holidays 0.74" aligns with the current library version.
31-121: French localization structure is correct.The empty msgstr values are intentional and proper for the source language file (fr_BJ). The msgids contain the appropriate French holiday names that match the implementation in the Benin class.
holidays/locale/en_US/LC_MESSAGES/BJ.po (3)
40-40: American English spelling is correct."Labor Day" is the proper US English spelling, correctly addressing previous feedback about British vs American spelling.
76-76: Armed Forces Day translation is clear and appropriate.The simplified "Armed Forces Day" translation is much clearer than the previous awkward phrasing.
32-121: English translations are comprehensive and accurate.All holiday names are properly translated to US English and cover the complete set of holidays implemented in the Benin class.
holidays/countries/benin.py (8)
20-70: Class structure and attributes are well-designed.The multiple inheritance pattern is appropriate for Benin's diverse holiday types. Class attributes correctly specify the country code, language support, categories, and start year based on legal references.
71-81: Constructor properly initializes all parent classes.The Islamic holidays integration with optional estimated labeling is a nice touch for handling uncertain dates.
87-94: Vodoun Festival logic handles the 2025 change correctly.The two-day holiday implementation for 2025+ (Thursday and Friday of the second week) properly reflects the new celebration format mentioned in the legal references.
96-102: Historical holiday periods are properly bounded.The 1980-1990 period for Martyrs' Day and Youth Day reflects the specific legal timeframes mentioned in the documentation.
103-122: Easter celebration changes are correctly implemented.The switch from Easter Sunday (pre-1991) to Easter Monday (1991+) and similar Whit Sunday/Monday changes properly reflect the legal evolution.
136-141: National Day date change is properly handled.The transition from November 30 (pre-1991) to August 1 (1991+) correctly implements the legal changes.
162-172: WORKDAY holidays are properly categorized.The separation of workday-specific holidays (Remembrance Day, People's Sovereignty Day, Women's Day) from public holidays follows the library's categorization pattern.
174-179: Country aliases are standard and appropriate.The BJ and BEN alias classes follow the library's pattern for providing alternative country code access.
tests/countries/test_benin.py (6)
1-18: Clean file structure and imports.The header, imports, and file organization follow the project standards correctly.
20-26: Well-structured class setup.The setup correctly configures the different holiday variants (standard, no estimated Islamic holidays, and workday categories) with appropriate year ranges.
28-29: Proper alias testing.Correctly tests the country code aliases BJ and BEN.
31-33: Good edge case coverage.Appropriately tests that no holidays exist before the start year (1976) and for workday categories before they were introduced (1989).
113-125: Ascension Day test looks good.The typo in the method name has been fixed and the test logic is correct.
228-267: Comprehensive localization testing.Both French (default) and English localizations are thoroughly tested with a good variety of holidays including estimated Islamic holidays.
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
|
Proposed change
Add Benin Holidays
Resolves: #1155
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green