Add French Southern Territories holidays #2442
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis pull request introduces support for the French Southern Territories in the library. It includes the addition of a new author in Changes
Assessment against linked issues
Suggested reviewers
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (8)
AUTHORS.md(1 hunks)README.md(2 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/french_southern_territories.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/TF.po(1 hunks)holidays/locale/fr/LC_MESSAGES/TF.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_french_southern_territories.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
holidays/countries/__init__.py (1)
holidays/countries/french_southern_territories.py (3)
FrenchSouthernTerritories(20-84)TF(87-88)ATF(91-92)
tests/countries/test_french_southern_territories.py (2)
tests/common.py (7)
TestCase(28-338)assertNoHoliday(244-246)assertHolidayDates(170-172)assertHoliday(150-152)assertNoHolidayName(273-275)assertHolidayName(195-199)assertLocalizedHolidays(327-338)holidays/countries/french_southern_territories.py (3)
FrenchSouthernTerritories(20-84)TF(87-88)ATF(91-92)
holidays/countries/french_southern_territories.py (4)
holidays/countries/france.py (1)
France(19-236)holidays/groups/christian.py (7)
ChristianHolidays(22-463)_add_easter_monday(259-268)_add_whit_monday(432-441)_add_ascension_thursday(119-127)_add_assumption_of_mary_day(148-164)_add_all_saints_day(97-105)_add_christmas_day(208-216)holidays/groups/international.py (4)
InternationalHolidays(18-220)_add_new_years_day(126-134)_add_labor_day(99-108)_add_world_war_two_victory_day(199-210)holidays/observed_holiday_base.py (1)
ObservedHolidayBase(101-239)
🔇 Additional comments (19)
AUTHORS.md (1)
86-86: Welcome to the project!The addition of your name to the AUTHORS.md file is correctly formatted and placed alphabetically in the list.
README.md (2)
102-102: Country count updated correctlyThe count has been properly incremented from 165 to 166 to reflect the addition of the French Southern Territories.
529-534: French Southern Territories entry added properlyThe entry is correctly formatted and includes all necessary information (country name, code, and supported languages). The entry is also placed in the correct alphabetical position.
holidays/countries/__init__.py (1)
70-70: Import statement added properlyThe import for the French Southern Territories module is correctly placed in alphabetical order and includes all necessary classes and constants.
holidays/registry.py (1)
79-79: Registry entry added properlyThe entry for French Southern Territories in the COUNTRIES dictionary follows the correct format and is alphabetically positioned between "france" and "gabon".
holidays/locale/en_US/LC_MESSAGES/TF.po (2)
15-29: The metadata and header information looks correct.The file contains all the necessary metadata for the localization file including project version, creation date, translator information, and language specifications.
30-77: Holiday translations look complete and accurate.All the holiday translations from French to English are appropriate and cover the full range of holidays implemented in the French Southern Territories class.
holidays/locale/fr/LC_MESSAGES/TF.po (1)
30-76: Empty msgstr fields are correct for French localization.Since the original holiday names (msgid) are already in French, leaving the translated strings (msgstr) empty is the correct approach for a French localization file.
holidays/countries/french_southern_territories.py (5)
13-18: Module imports look appropriate and comprehensive.The imports include all necessary components: gettext for translation, France as the parent class, and required holiday group classes.
20-32: Class definition and attributes are properly structured.The class inherits from appropriate base classes and correctly defines country code, parent entity relationship, language support, and start year. The comment on line 31 explaining the reasoning for using France's start year is helpful.
34-38: Constructor implementation is correct.The constructor properly initializes parent classes in the appropriate order, ensuring the inheritance chain works correctly.
39-84: Holiday population method follows the expected pattern.The
_populate_public_holidaysmethod correctly implements holiday definitions following the same pattern and year-based conditions as the France class, which is appropriate since these are French territories.
87-93: Empty alias classes look correct.The TF and ATF alias classes are properly defined. These empty classes inheriting from the main class follow the pattern used elsewhere in the codebase.
tests/countries/test_french_southern_territories.py (6)
13-17: Appropriate imports for the test class.The test file imports the necessary components: TestCase from unittest, all relevant classes from the french_southern_territories module, and CommonCountryTests base class.
19-26: Test class setup is correctly implemented.The TestTF class properly extends from CommonCountryTests and TestCase. The setUpClass method initializes with appropriate parameters, and the test_country_aliases method verifies the alias classes.
27-29: Pre-1802 tests verify correct historical behavior.The test correctly verifies that religious holidays (Assumption, All Saints, Christmas) are not observed before 1802, which matches the implementation in the French Southern Territories class.
30-60: Year-specific holiday tests are comprehensive.Tests for 2017 and 2022 verify that all expected holidays are present on the correct dates, providing good coverage of the holiday calculation logic.
62-104: Individual holiday tests verify correct year ranges.Tests for specific holidays (New Year's Day, Labor Day, Victory Day, National Day, Armistice Day, Pentecost Monday) properly verify both the existence of holidays in expected years and their absence in years before they were established.
105-134: Localization tests verify correct translations.The tests for default (French) and English (en_US) localization verify that all holiday names are correctly translated in both languages, ensuring the localization files are properly integrated.
Improves quality code with Copilot recommendation Co-authored-by: Copilot <[email protected]> Signed-off-by: Lucca Augusto <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/countries/test_french_southern_territories.py (1)
80-80: Previous issue has been addressed correctly.This line correctly implements the suggested fix from the previous review by using a list comprehension with proper unpacking.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/countries/test_french_southern_territories.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/countries/test_french_southern_territories.py (2)
tests/common.py (7)
TestCase(28-338)assertAliases(121-130)assertNoHoliday(244-246)assertHolidayDates(170-172)assertHoliday(150-152)assertNoHolidayName(273-275)assertLocalizedHolidays(327-338)holidays/countries/french_southern_territories.py (3)
FrenchSouthernTerritories(20-84)TF(87-88)ATF(91-92)
🔇 Additional comments (2)
tests/countries/test_french_southern_territories.py (2)
1-134: Test coverage looks comprehensive and well-structured.The test file provides thorough coverage for the new
FrenchSouthernTerritoriesholiday class. It effectively tests historical introduction dates, name changes, specific years, and proper localization for both French and English.
97-104: Good test for Pentecost Monday exceptions.This test properly verifies that Pentecost Monday wasn't observed during 2005-2007 but was observed in surrounding years. This matches the implementation in the
FrenchSouthernTerritoriesclass.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2442 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 213 214 +1
Lines 13510 13527 +17
Branches 1928 1928
=========================================
+ Hits 13510 13527 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Since it's an overseas territory of France, it should be implemented similar to the US dependent territories (Guam, Puerto Rico, etc.). For localization, the approach is similar to Isle of Man. |
Proposed change
Add French Southern Territories holidays.
Resolves #2434.
Type of change
Checklist
make check, all checks and tests are green