Add Christmas Island holidays #2623
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughAdded support for Christmas Island holidays by introducing a new country module, registry updates, localization files, and comprehensive test suite, expanding the library's coverage to 210 countries. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were identified in this pull request. Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🧰 Additional context used🧠 Learnings (4)📓 Common learningsholidays/registry.py (12)README.md (29)holidays/countries/__init__.py (16)🧬 Code Graph Analysis (1)holidays/countries/__init__.py (1)
🪛 LanguageToolREADME.md[grammar] ~108-~108: Use correct spacing (QB_NEW_EN_OTHER_ERROR_IDS_5) 🔇 Additional comments (3)
✨ 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. 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
🔭 Outside diff range comments (1)
holidays/locale/en_CX/LC_MESSAGES/CX.po (1)
97-106: 🧹 Nitpick (assertive)Consider cleaning up legacy comments.
The commented fallback entries (#~ msgid) appear unused; removing them could streamline the file.
📜 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/christmas_islands.py(1 hunks)holidays/locale/en_CX/LC_MESSAGES/CX.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/CX.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_christmas_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_US/LC_MESSAGES/CX.po (1)
Learnt from: KJhellico
PR: vacanza/holidays#2388
File: holidays/locale/en_CI/LC_MESSAGES/CI.po:88-88
Timestamp: 2025-03-30T18:25:07.087Z
Learning: In the holidays library, localization files have a specific structure: message comments are in standard English (en_US) describing the holiday, while actual translations (msgstr) should use the locale-specific terminology (e.g., en_CI for Ivory Coast English). For example, "Night of Power" in standard English is translated as "Lailatou-Kadr" in Ivory Coast English.
tests/countries/test_christmas_islands.py (3)
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
Learnt from: 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 (2)
holidays/countries/__init__.py (1)
holidays/countries/christmas_islands.py (3)
ChristmasIslands(32-114)CX(155-156)CXR(159-160)
tests/countries/test_christmas_islands.py (2)
tests/common.py (7)
TestCase(28-338)CommonCountryTests(356-374)assertAliases(121-130)assertNoHolidays(292-294)assertHolidayName(195-199)assertNoNonObservedHoliday(248-250)assertLocalizedHolidays(327-338)holidays/countries/christmas_islands.py (3)
ChristmasIslands(32-114)CX(155-156)CXR(159-160)
🪛 Pylint (3.3.7)
tests/countries/test_christmas_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestChristmasIslands.setUpClass' method
(W0221)
[convention] 26-26: Missing function or method docstring
(C0116)
[convention] 29-29: Missing function or method docstring
(C0116)
[convention] 32-32: Missing function or method docstring
(C0116)
[convention] 45-45: Missing function or method docstring
(C0116)
[convention] 57-57: Missing function or method docstring
(C0116)
[convention] 70-70: Missing function or method docstring
(C0116)
[convention] 83-83: Missing function or method docstring
(C0116)
[convention] 96-96: Missing function or method docstring
(C0116)
[convention] 109-109: Missing function or method docstring
(C0116)
[convention] 122-122: Missing function or method docstring
(C0116)
[convention] 144-144: Missing function or method docstring
(C0116)
[convention] 157-157: Missing function or method docstring
(C0116)
[convention] 169-169: Missing function or method docstring
(C0116)
[convention] 182-182: Missing function or method docstring
(C0116)
[convention] 203-203: Missing function or method docstring
(C0116)
[convention] 224-224: Missing function or method docstring
(C0116)
[convention] 246-246: Missing function or method docstring
(C0116)
holidays/countries/christmas_islands.py
[convention] 45-45: Line too long (169/100)
(C0301)
[convention] 46-46: Line too long (189/100)
(C0301)
[convention] 167-167: Line too long (193/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[refactor] 32-32: Too many ancestors (8/7)
(R0901)
[convention] 117-117: Missing class docstring
(C0115)
[convention] 155-155: Missing class docstring
(C0115)
[refactor] 155-155: Too many ancestors (9/7)
(R0901)
[convention] 159-159: Missing class docstring
(C0115)
[refactor] 159-159: Too many ancestors (9/7)
(R0901)
[refactor] 163-163: Too few public methods (0/2)
(R0903)
🔇 Additional comments (8)
holidays/countries/__init__.py (1)
53-53: Expose Christmas Islands holidays.
ImportingChristmasIslands,CX, andCXRcorrectly integrates the new country into the package namespace.README.md (1)
416-421: Add Christmas Islands entry to supported countries.
The new table row correctly lists "Christmas Islands" with codeCXand supported languages en_CX, en_US.holidays/registry.py (1)
61-61: Register Christmas Islands in COUNTRIES mapping.
The key"christmas_islands"and its tuple("ChristmasIslands", "CX", "CXR")follow naming and sorting conventions.holidays/locale/en_US/LC_MESSAGES/CX.po (2)
22-22: Validate header locale.
The headerLanguage: en_USaligns with the file location and intended locale.
30-95: Verify message translations.
Allmsgstrvalues correctly mirror theirmsgidcounterparts and require no adjustments.holidays/locale/en_CX/LC_MESSAGES/CX.po (1)
30-96: Check translation entries.
Allmsgid/msgstrpairs mirror the holiday identifiers and match English (Christmas Island) defaults.tests/countries/test_christmas_islands.py (1)
1-267: Well-structured test implementation!The test file follows all project conventions and provides comprehensive coverage for Christmas Islands holidays.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestChristmasIslands.setUpClass' method
(W0221)
[convention] 26-26: Missing function or method docstring
(C0116)
[convention] 29-29: Missing function or method docstring
(C0116)
[convention] 32-32: Missing function or method docstring
(C0116)
[convention] 45-45: Missing function or method docstring
(C0116)
[convention] 57-57: Missing function or method docstring
(C0116)
[convention] 70-70: Missing function or method docstring
(C0116)
[convention] 83-83: Missing function or method docstring
(C0116)
[convention] 96-96: Missing function or method docstring
(C0116)
[convention] 109-109: Missing function or method docstring
(C0116)
[convention] 122-122: Missing function or method docstring
(C0116)
[convention] 144-144: Missing function or method docstring
(C0116)
[convention] 157-157: Missing function or method docstring
(C0116)
[convention] 169-169: Missing function or method docstring
(C0116)
[convention] 182-182: Missing function or method docstring
(C0116)
[convention] 203-203: Missing function or method docstring
(C0116)
[convention] 224-224: Missing function or method docstring
(C0116)
[convention] 246-246: Missing function or method docstring
(C0116)
holidays/countries/christmas_islands.py (1)
1-177: Clean holiday implementation!The module properly implements all Christmas Islands holidays with correct inheritance and initialization patterns.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 45-45: Line too long (169/100)
(C0301)
[convention] 46-46: Line too long (189/100)
(C0301)
[convention] 167-167: Line too long (193/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[refactor] 32-32: Too many ancestors (8/7)
(R0901)
[convention] 117-117: Missing class docstring
(C0115)
[convention] 155-155: Missing class docstring
(C0115)
[refactor] 155-155: Too many ancestors (9/7)
(R0901)
[convention] 159-159: Missing class docstring
(C0115)
[refactor] 159-159: Too many ancestors (9/7)
(R0901)
[refactor] 163-163: Too few public methods (0/2)
(R0903)
arkid15r
requested changes
Jun 11, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)holidays/countries/__init__.py(1 hunks)holidays/registry.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (1)
holidays/countries/christmas_islands.py (3)
ChristmasIslands(32-114)CX(155-156)CXR(159-160)
🔇 Additional comments (2)
holidays/countries/__init__.py (1)
53-53: Imported Christmas Islands module correctly.
The new import ofChristmasIslands,CX, andCXRis correctly placed alphabetically after China and before Cocos Islands, exposing the class and its aliases.holidays/registry.py (1)
61-61: Registered Christmas Islands in the registry.
The key"christmas_islands"with("ChristmasIslands", "CX", "CXR")aligns with the module name and class names, and it’s appropriately ordered between China and Cocos Islands.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2623 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 258 259 +1
Lines 15468 15525 +57
Branches 2105 2110 +5
=========================================
+ Hits 15468 15525 +57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
holidays/locale/en_CX/LC_MESSAGES/CX.po (1)
22-22:⚠️ Potential issueHeader still shows
en_US; should been_CX.
Same issue was raised earlier and is still unresolved.-"Language: en_US\n" +"Language: en_CX\n"holidays/countries/christmas_islands.py (1)
112-114: 🧹 Nitpick (assertive)Still missing explanatory comment for 2025 Eid al-Adha special case.
Adding a one-liner keeps maintainability high.- for dt in self._add_eid_al_adha_day(tr("Eid al-Adha")): - if self._year != 2025: - self._add_observed(dt) + # Eid al-Adha is statically observed on 6 Jun 2025; skip default observation that year. + for dt in self._add_eid_al_adha_day(tr("Eid al-Adha")): + if self._year != 2025: + self._add_observed(dt)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/christmas_islands.py(1 hunks)holidays/locale/en_CX/LC_MESSAGES/CX.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/CX.po(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_US/LC_MESSAGES/CX.po (1)
Learnt from: KJhellico
PR: vacanza/holidays#2388
File: holidays/locale/en_CI/LC_MESSAGES/CI.po:88-88
Timestamp: 2025-03-30T18:25:07.087Z
Learning: In the holidays library, localization files have a specific structure: message comments are in standard English (en_US) describing the holiday, while actual translations (msgstr) should use the locale-specific terminology (e.g., en_CI for Ivory Coast English). For example, "Night of Power" in standard English is translated as "Lailatou-Kadr" in Ivory Coast English.
holidays/countries/christmas_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:249-253
Timestamp: 2025-05-10T04:02:13.815Z
Learning: Holiday name comments directly above tr() function calls in the holidays package should only contain the holiday name itself (e.g., "# Independence Day.") without any additional context, dates, or historical information.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.753Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
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.").
🪛 Pylint (3.3.7)
holidays/countries/christmas_islands.py
[convention] 45-45: Line too long (169/100)
(C0301)
[convention] 46-46: Line too long (189/100)
(C0301)
[convention] 167-167: Line too long (193/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[refactor] 32-32: Too many ancestors (8/7)
(R0901)
[convention] 117-117: Missing class docstring
(C0115)
[convention] 155-155: Missing class docstring
(C0115)
[refactor] 155-155: Too many ancestors (9/7)
(R0901)
[convention] 159-159: Missing class docstring
(C0115)
[refactor] 159-159: Too many ancestors (9/7)
(R0901)
[refactor] 163-163: Too few public methods (0/2)
(R0903)
🔇 Additional comments (2)
holidays/locale/en_US/LC_MESSAGES/CX.po (1)
15-96: Translations look sound.
No functional or formatting concerns spotted in this file.holidays/countries/christmas_islands.py (1)
84-97: 💡 Verification agent🧩 Analysis chain
Verify helper methods exist.
Methods_add_holiday_4th_mon_of_marand_add_holiday_oct_6are used but aren’t common helpers in many country files. Confirm they are implemented in the codebase to avoidAttributeError.
🏁 Script executed:
#!/bin/bash # Search for helper implementations. rg --line-number "_add_holiday_(4th_mon_of_mar|oct_6)\(" -A1Length of output: 622
Let’s double-check that these helpers are actually defined in the codebase:
🏁 Script executed:
#!/bin/bash # Search for definitions of the helper methods. rg --line-number "def _add_holiday_4th_mon_of_mar" -n rg --line-number "def _add_holiday_oct_6" -nLength of output: 102
Undefined helper methods detected
I couldn’t find any definitions for
_add_holiday_4th_mon_of_maror_add_holiday_oct_6in the codebase. Calling these will raise anAttributeError.• In holidays/countries/christmas_islands.py (lines 84–97):
- _add_holiday_4th_mon_of_mar
- _add_holiday_oct_6
Please add or import these helper implementations (e.g. in the base calendar class) before using them.
⛔ Skipped due to learnings
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.
PPsyrius
reviewed
Jun 12, 2025
KJhellico
reviewed
Jun 12, 2025
|
@PPsyrius @KJhellico i have done the above mentioned changes.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
README.md (1)
416-420: Trim leading space before the default language tag.
<td> <strong>en_CX</strong>, en_US</td>contains an extra space after the opening<td>that breaks the visual alignment of the table and is inconsistent with the surrounding rows.-<td> <strong>en_CX</strong>, en_US</td> +<td><strong>en_CX</strong>, en_US</td>holidays/locale/en_CX/LC_MESSAGES/CX.po (1)
20-24: Header still reportsen_US; switch toen_CX.Same issue as flagged earlier: the
Language:header must match the locale directory.-"Language: en_US\n" +"Language: en_CX\n"
📜 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/christmas_island.py(1 hunks)holidays/locale/en_CX/LC_MESSAGES/CX.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/CX.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_christmas_island.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_US/LC_MESSAGES/CX.po (1)
Learnt from: KJhellico
PR: vacanza/holidays#2388
File: holidays/locale/en_CI/LC_MESSAGES/CI.po:88-88
Timestamp: 2025-03-30T18:25:07.087Z
Learning: In the holidays library, localization files have a specific structure: message comments are in standard English (en_US) describing the holiday, while actual translations (msgstr) should use the locale-specific terminology (e.g., en_CI for Ivory Coast English). For example, "Night of Power" in standard English is translated as "Lailatou-Kadr" in Ivory Coast English.
tests/countries/test_christmas_island.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:85-86
Timestamp: 2025-04-05T04:29:38.042Z
Learning: For testing holiday implementations in the vacanza/holidays repository, recommend using `from tests.common import CommonCountryTests` as the base class instead of directly using `unittest.TestCase` to maintain consistency with project conventions and leverage common test utilities.
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
🪛 Flake8 (7.2.0)
holidays/countries/__init__.py
[error] 53-53: 'holidays.countries.christmas_island.ChristmasIsland' imported but unused
(F401)
[error] 53-53: 'holidays.countries.christmas_island.CX' imported but unused
(F401)
[error] 53-53: 'holidays.countries.christmas_island.CXR' imported but unused
(F401)
🪛 Pylint (3.3.7)
tests/countries/test_christmas_island.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestChristmasIsland.setUpClass' method
(W0221)
[convention] 26-26: Missing function or method docstring
(C0116)
[convention] 29-29: Missing function or method docstring
(C0116)
[convention] 32-32: Missing function or method docstring
(C0116)
[convention] 45-45: Missing function or method docstring
(C0116)
[convention] 57-57: Missing function or method docstring
(C0116)
[convention] 70-70: Missing function or method docstring
(C0116)
[convention] 83-83: Missing function or method docstring
(C0116)
[convention] 96-96: Missing function or method docstring
(C0116)
[convention] 109-109: Missing function or method docstring
(C0116)
[convention] 122-122: Missing function or method docstring
(C0116)
[convention] 143-143: Missing function or method docstring
(C0116)
[convention] 156-156: Missing function or method docstring
(C0116)
[convention] 168-168: Missing function or method docstring
(C0116)
[convention] 181-181: Missing function or method docstring
(C0116)
[convention] 202-202: Missing function or method docstring
(C0116)
[convention] 223-223: Missing function or method docstring
(C0116)
[convention] 245-245: Missing function or method docstring
(C0116)
holidays/countries/christmas_island.py
[convention] 45-45: Line too long (169/100)
(C0301)
[convention] 46-46: Line too long (189/100)
(C0301)
[convention] 47-47: Line too long (208/100)
(C0301)
[convention] 48-48: Line too long (202/100)
(C0301)
[convention] 49-49: Line too long (240/100)
(C0301)
[convention] 50-50: Line too long (214/100)
(C0301)
[convention] 51-51: Line too long (252/100)
(C0301)
[convention] 52-52: Line too long (256/100)
(C0301)
[convention] 53-53: Line too long (233/100)
(C0301)
[convention] 54-54: Line too long (248/100)
(C0301)
[convention] 55-55: Line too long (256/100)
(C0301)
[convention] 56-56: Line too long (249/100)
(C0301)
[convention] 57-57: Line too long (186/100)
(C0301)
[convention] 58-58: Line too long (186/100)
(C0301)
[convention] 59-59: Line too long (218/100)
(C0301)
[convention] 60-60: Line too long (175/100)
(C0301)
[convention] 61-61: Line too long (194/100)
(C0301)
[convention] 62-62: Line too long (175/100)
(C0301)
[convention] 63-63: Line too long (194/100)
(C0301)
[convention] 190-190: Line too long (193/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[refactor] 32-32: Too many ancestors (8/7)
(R0901)
[convention] 141-141: Missing class docstring
(C0115)
[convention] 178-178: Missing class docstring
(C0115)
[refactor] 178-178: Too many ancestors (9/7)
(R0901)
[convention] 182-182: Missing class docstring
(C0115)
[refactor] 182-182: Too many ancestors (9/7)
(R0901)
[refactor] 186-186: Too few public methods (0/2)
(R0903)
🔇 Additional comments (9)
holidays/registry.py (1)
61-63: Looks good – registry entry is alphabetically correct.
"christmas_island": ("ChristmasIsland", "CX", "CXR"),is inserted in the expected alphabetical slot between China and Cocos Islands and uses unique ISO codes. No further action required.holidays/countries/__init__.py (1)
52-55: Verify__all__update.The new import is fine and the file is already whitelisted with
# ruff: noqa: F401, but remember to appendChristmasIsland,CX, andCXRto the__all__tuple at the bottom of the file so thatfrom holidays.countries import *exposes them.holidays/locale/en_US/LC_MESSAGES/CX.po (1)
83-90: Translations look sensible.Holiday names are properly localised to en-US (“Eid al-Fitr”, “Eid al-Adha”) while keeping Malay msgids, matching the library’s localisation convention.
No issues spotted.tests/countries/test_christmas_island.py (3)
15-24: Well-structured test setup following project conventions.Good use of
CommonCountryTestsas the base class and proper initialization of test instances with comprehensive year ranges.
34-34: Consistent year range pattern across test methods.Good adherence to project convention of recreating year ranges within each test method.
Also applies to: 72-72, 111-111
133-141: Thorough Islamic holiday testing with estimation handling.Excellent approach testing Islamic holidays with both estimated and non-estimated instances.
Also applies to: 192-200
holidays/countries/christmas_island.py (3)
101-106: Verify the Labour Day date variation logic.The switching between 3rd and 4th Monday of March for specific years seems unusual. Please confirm this matches official proclamations.
136-138: Special handling for 2025 Eid al-Adha looks correct.The logic properly skips automatic observance for 2025 since it's handled as a special observed holiday on June 6.
Also applies to: 198-199
121-125: Smart Boxing Day observance rule implementation.The combined rule ensures proper handling when Boxing Day falls on various days of the week.
|
You need to merge latest |
Signed-off-by: Ansh Anand <ansh.a.3112@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
README.md(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: holidays/countries/christmas_island.py:110-112
Timestamp: 2025-07-09T20:27:37.718Z
Learning: In Christmas Island, ANZAC Day (April 25) follows the same observed holiday rules as other holidays, using the SAT_SUN_TO_NEXT_MON rule to move to Monday when it falls on a weekend. The implementation correctly uses `_add_observed` wrapper around `_add_anzac_day`.
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: tests/countries/test_christmas_island.py:136-146
Timestamp: 2025-07-09T21:16:35.116Z
Learning: In Christmas Island's holiday implementation, the test_christmas_day method cannot use assertNoNonObservedHoliday because in some years observed Christmas Day overlaps with Boxing Day when both holidays are moved due to weekend conflicts, causing the standard non-observed holiday check to fail inappropriately.
Learnt from: KJhellico
PR: vacanza/holidays#2631
File: tests/countries/test_sint_maarten.py:62-0
Timestamp: 2025-06-14T21:12:07.224Z
Learning: KJhellico prefers to focus on completing and reviewing the main holiday implementation code before doing detailed reviews of the corresponding test files.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:171-183
Timestamp: 2025-03-29T15:15:05.919Z
Learning: In the Fiji holidays implementation, the maintainers are aware of the need to extend the MAWLID_DATES dictionary beyond 2025 when future official references become available, and will do so when appropriate. No suggestions about extending this dictionary should be made in future reviews.
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: 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.").
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:131-131
Timestamp: 2025-07-08T10:20:04.047Z
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.
Learnt from: KJhellico
PR: vacanza/holidays#2483
File: holidays/countries/turks_and_caicos_islands.py:117-118
Timestamp: 2025-05-13T13:23:11.375Z
Learning: The holidays library uses `_add_christmas_day_two` method to add Boxing Day holiday, not `_add_boxing_day`.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2601
File: README.md:108-108
Timestamp: 2025-06-09T19:39:38.818Z
Learning: The correct way to count supported countries in the holidays library is by counting unique country module files in holidays/countries/ (excluding __init__.py) or by counting entries in the COUNTRIES registry, not by counting individual class definitions which include alias classes.
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: holidays/ical.py:53-80
Timestamp: 2025-03-26T08:55:08.917Z
Learning: The holidays library intentionally limits language code support to ISO 639-1 and ISO 639-2 standards (2-3 letter language codes with optional region) to maintain consistency with other modules like `common_holidays` and `financial_holidays`, rather than implementing full RFC 5646 compliance.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.641Z
Learning: Library-wide holiday patterns and their optimizations should be handled at the base class level (like InternationalHolidays) rather than documenting workarounds in individual country modules. This maintains separation of concerns and avoids documentation duplication.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:1-12
Timestamp: 2025-04-24T04:59:38.436Z
Learning: The holidays library uses a standard file header across country implementation files that includes copyright information, authors, website, and license details. New country implementation files should follow this same header format.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:1-12
Timestamp: 2025-04-24T04:59:38.436Z
Learning: The holidays library uses a standard file header across country implementation files that includes copyright information, authors, website, and license details. New country implementation files should follow this same header format.
README.md (29)
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: README.md:562-568
Timestamp: 2025-06-06T16:02:09.910Z
Learning: The README.md country table displays ISO 3166-1 alpha-2 codes only in the "Code" column. Alpha-3 codes or country aliases should not be included in this table format, maintaining consistency with all other country entries.
Learnt from: PPsyrius
PR: vacanza/holidays#2638
File: holidays/countries/svalbard_and_jan_mayen.py:1-12
Timestamp: 2025-06-16T11:42:28.293Z
Learning: In the holidays library codebase, country implementation files in holidays/countries/ follow a standard convention of NOT having module-level docstrings. They start with the license header comment block, followed by imports, then class definitions. This is consistent across all country implementations like austria.py, belgium.py, canada.py, etc.
Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: holidays/ical.py:53-80
Timestamp: 2025-03-26T08:55:08.917Z
Learning: The holidays library intentionally limits language code support to ISO 639-1 and ISO 639-2 standards (2-3 letter language codes with optional region) to maintain consistency with other modules like `common_holidays` and `financial_holidays`, rather than implementing full RFC 5646 compliance.
Learnt from: KJhellico
PR: vacanza/holidays#2629
File: holidays/locale/en_US/LC_MESSAGES/NA.po:17-28
Timestamp: 2025-06-14T11:00:25.835Z
Learning: In the holidays library localization files, the X-Source-Language field should contain the value that matches the country's default language setting (e.g., if a country class has default_language = "en_NA", then X-Source-Language should be "en_NA").
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/countries/senegal.py:104-110
Timestamp: 2025-06-05T17:22:43.114Z
Learning: In the holidays library codebase, country alias classes (like SN, SEN for Senegal) follow a consistent convention of having no docstrings and just containing "pass". This pattern is used across all countries - examples include BR/BRA for Brazil, CA/CAN for Canada, DE/DEU for Germany, etc. Static analysis tools may flag missing docstrings, but the established codebase convention is to keep these alias classes minimal.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:28-33
Timestamp: 2025-04-02T04:51:17.842Z
Learning: The library convention for country code aliases (like two-letter ISO 3166-1 alpha-2 and three-letter ISO 3166-1 alpha-3 codes) is to implement them as simple pass-through classes without docstrings.
Learnt from: PPsyrius
PR: vacanza/holidays#2362
File: holidays/ical.py:46-67
Timestamp: 2025-03-23T10:44:39.256Z
Learning: The ICalExporter class in the holidays library intentionally restricts language codes to two-letter ISO 639-1 codes as required by iCalendar specifications. This validation is comprehensively tested in test_language_code.
Learnt from: PPsyrius
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:25-25
Timestamp: 2025-04-02T04:54:07.204Z
Learning: For English-speaking countries in the holidays library (like Trinidad and Tobago), the `default_language` and `supported_languages` parameters are optional. If languages are added for such countries, ensure that `en_US` is included in the list alongside the regional code variant (e.g., `en_TT` for Trinidad and Tobago).
Learnt from: KJhellico
PR: vacanza/holidays#2614
File: README.md:108-108
Timestamp: 2025-06-14T20:13:29.536Z
Learning: In the vacanza/holidays repository, the team prefers to manually update the country count in README.md when adding new countries, with tests in place to verify the count stays accurate. They are satisfied with this approach and do not want it automated.
Learnt from: KJhellico
PR: vacanza/holidays#2375
File: README.md:224-226
Timestamp: 2025-03-26T22:41:55.075Z
Learning: The countries table in README.md uses HTML format consistently throughout, not Markdown.
Learnt from: KJhellico
PR: vacanza/holidays#2483
File: holidays/registry.py:177-177
Timestamp: 2025-04-18T21:13:55.589Z
Learning: In the holidays/registry.py file, COUNTRIES dictionary entries should use the class name (without spaces) as the first element of the tuple (e.g., "TurksAndCaicosIslands"), not the human-readable country name with spaces.
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:21:59.302Z
Learning: In the Saint Vincent and the Grenadines holiday tests, for holidays without observed rules that only require a single assertHolidayName call, pass the holiday name directly as a string literal rather than storing it in a variable first for cleaner, more concise code.
Learnt from: KJhellico
PR: vacanza/holidays#2614
File: holidays/countries/guyana.py:114-116
Timestamp: 2025-06-14T20:36:17.239Z
Learning: In the holidays codebase, message comments should always use en_US spelling conventions, while translatable strings can use appropriate local spellings for the target country/region.
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: tests/countries/test_guinea.py:237-239
Timestamp: 2025-04-02T18:38:35.164Z
Learning: In the vacanza/holidays project, the method assertLocalizedHolidays in country test files should be called with positional parameters rather than named parameters to maintain consistency with the rest of the codebase.
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: tests/countries/test_christmas_island.py:136-146
Timestamp: 2025-07-09T21:16:35.116Z
Learning: In Christmas Island's holiday implementation, the test_christmas_day method cannot use assertNoNonObservedHoliday because in some years observed Christmas Day overlaps with Boxing Day when both holidays are moved due to weekend conflicts, causing the standard non-observed holiday check to fail inappropriately.
Learnt from: PPsyrius
PR: vacanza/holidays#2676
File: holidays/countries/tunisia.py:25-25
Timestamp: 2025-06-25T10:12:30.448Z
Learning: The holidays library intentionally duplicates label strings like `estimated_label` across country modules rather than centralizing them. This duplication is the established library-wide approach and should not be suggested for refactoring.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2629
File: holidays/locale/en_US/LC_MESSAGES/NA.po:17-28
Timestamp: 2025-06-14T10:58:16.431Z
Learning: In the holidays library localization system, different variants of the same language (like en_NA and en_US) can serve as source and target languages, with X-Source-Language correctly indicating the regional variant that serves as the original text.
Learnt from: KJhellico
PR: vacanza/holidays#2601
File: README.md:108-108
Timestamp: 2025-06-09T19:39:38.818Z
Learning: The correct way to count supported countries in the holidays library is by counting unique country module files in holidays/countries/ (excluding __init__.py) or by counting entries in the COUNTRIES registry, not by counting individual class definitions which include alias classes.
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: holidays/countries/christmas_island.py:110-112
Timestamp: 2025-07-09T20:27:37.718Z
Learning: In Christmas Island, ANZAC Day (April 25) follows the same observed holiday rules as other holidays, using the SAT_SUN_TO_NEXT_MON rule to move to Monday when it falls on a weekend. The implementation correctly uses `_add_observed` wrapper around `_add_anzac_day`.
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: holidays/countries/christmas_island.py:104-109
Timestamp: 2025-07-08T20:39:56.315Z
Learning: In Christmas Island, Easter Monday is not a public holiday, unlike other Australian territories. The Christmas Island public holidays are specifically defined in official proclamations and do not include Easter Monday.
Learnt from: PPsyrius
PR: vacanza/holidays#2407
File: holidays/locale/en_TL/LC_MESSAGES/TL.po:42-44
Timestamp: 2025-04-03T05:58:00.033Z
Learning: The en_TL (English for Timor-Leste) localization for holidays intentionally uses "World" instead of "International" (e.g., "World Labour Day") to maintain accuracy with official Timor-Leste government sources, even though code comments and other localizations like en_US use "International".
Learnt from: PPsyrius
PR: vacanza/holidays#2407
File: holidays/locale/en_TL/LC_MESSAGES/TL.po:42-44
Timestamp: 2025-04-03T05:58:00.033Z
Learning: The en_TL (English for Timor-Leste) localization for holidays intentionally uses "World" instead of "International" (e.g., "World Labour Day") to maintain accuracy with official Timor-Leste government sources, even though code comments and other localizations like en_US use "International".
Learnt from: KJhellico
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:75-76
Timestamp: 2025-04-05T09:44:39.998Z
Learning: In the holidays library, en_US locale is the standard for localization comments (using American English spelling like "Labor"), while the actual holiday names in code use the appropriate locale-specific spelling (like "Labour" for British English locales such as en_TT).
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/locale/th/LC_MESSAGES/AX.po:15-27
Timestamp: 2025-05-10T04:35:01.221Z
Learning: The localization (.po) files for Åland Islands (AX) are intentionally left as stubs without translations because Åland Islands is implemented as a subdivision of Finland (code "01"). Holiday translations are handled through the Finland (FI) localization files instead.
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/french_southern_territories.py:41-44
Timestamp: 2025-06-18T10:07:58.780Z
Learning: Territorial holiday classes that inherit from parent countries (like HolidaysAX from Finland, HolidaysSJ from Norway, HolidaysTF from France) follow a standard pattern of silently overriding self.subdiv in their _populate_public_holidays() method without validation, as this ensures they always use the correct subdivision code for their territory regardless of user input.
Learnt from: KJhellico
PR: vacanza/holidays#2388
File: holidays/locale/en_CI/LC_MESSAGES/CI.po:88-88
Timestamp: 2025-03-30T18:25:07.087Z
Learning: In the holidays library, localization files have a specific structure: message comments are in standard English (en_US) describing the holiday, while actual translations (msgstr) should use the locale-specific terminology (e.g., en_CI for Ivory Coast English). For example, "Night of Power" in standard English is translated as "Lailatou-Kadr" in Ivory Coast English.
Learnt from: PPsyrius
PR: vacanza/holidays#2638
File: holidays/countries/svalbard_and_jan_mayen.py:26-32
Timestamp: 2025-06-16T11:46:35.303Z
Learning: When creating country holiday aliases that inherit from parent countries (like Svalbard and Jan Mayen from Norway, or Åland from Finland), it's common and acceptable to hardcode a specific subdivision code since the subdivision codes for the parent country yield identical results.
🪛 LanguageTool
README.md
[grammar] ~108-~108: Use correct spacing
Context: ...y to refer to a country is by using its [ISO 3166-1 alpha-2 code](https://en.wikipedia.org/...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (1)
README.md (1)
430-435: New Christmas Island row looks correctAlphabetical placement, ISO-2 code
CX, and language list<strong>en_CX</strong>, en_USall match project conventions.
Nice catch removing the stray space KJhellico flagged earlier.
|
arkid15r
reviewed
Jul 10, 2025
arkid15r
approved these changes
Jul 10, 2025
This was referenced Jul 21, 2025
Merged
Merged
Merged
This was referenced Jul 24, 2025
Merged
This was referenced Jul 31, 2025
Merged
This was referenced Aug 10, 2025
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Added Christmas Island holidays
Resolved #2428
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green