-
-
Notifications
You must be signed in to change notification settings - Fork 539
Add Cook Islands holidays #2582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes introduce support for Cook Islands holidays by adding a new country module, updating the registry, localization files, documentation, and comprehensive tests. The updates ensure Cook Islands holidays, subdivisions, and localizations are included in the holidays library. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)holidays/countries/cook_islands.py (1)
🪛 Pylint (3.3.7)holidays/countries/cook_islands.py[convention] 29-29: Line too long (207/100) (C0301) [convention] 30-30: Line too long (194/100) (C0301) [convention] 31-31: Line too long (214/100) (C0301) [convention] 32-32: Line too long (217/100) (C0301) [convention] 33-33: Line too long (112/100) (C0301) [convention] 34-34: Line too long (249/100) (C0301) [convention] 35-35: Line too long (201/100) (C0301) [convention] 36-36: Line too long (123/100) (C0301) [convention] 37-37: Line too long (114/100) (C0301) [convention] 1-1: Missing module docstring (C0114) [convention] 143-143: Missing class docstring (C0115) [convention] 147-147: Missing class docstring (C0115) 🔇 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:
@coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 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/cook_islands.py
(1 hunks)holidays/locale/en_CK/LC_MESSAGES/CK.po
(1 hunks)holidays/locale/en_US/LC_MESSAGES/CK.po
(1 hunks)holidays/registry.py
(1 hunks)tests/countries/test_cook_islands.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_CK/LC_MESSAGES/CK.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/locale/en_US/LC_MESSAGES/CK.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.
🧬 Code Graph Analysis (2)
tests/countries/test_cook_islands.py (2)
tests/common.py (8)
TestCase
(28-338)CommonCountryTests
(356-374)assertAliases
(121-130)assertNoHolidays
(292-294)assertHolidayName
(195-199)assertNoNonObservedHoliday
(248-250)assertNoHolidayName
(273-275)assertLocalizedHolidays
(327-338)holidays/countries/cook_islands.py (3)
CookIslands
(23-162)CK
(165-166)COK
(169-170)
holidays/countries/__init__.py (1)
holidays/countries/cook_islands.py (3)
CookIslands
(23-162)CK
(165-166)COK
(169-170)
🔇 Additional comments (18)
holidays/countries/__init__.py (1)
55-55
: Cook Islands import integrated nicely
TheCookIslands, CK, COK
import is alphabetically positioned and matches the class names incook_islands.py
.holidays/registry.py (1)
63-63
: Registry entry added for Cook Islands
The"cook_islands": ("CookIslands", "CK", "COK")
mapping is correctly placed in alphabetical order and matches the module and codes.README.md (1)
430-436
: Document Cook Islands in the supported countries table
The new<tr>
is inserted between Congo and Costa Rica, lists accurate subdivisions and languages consistent with the code.holidays/locale/en_CK/LC_MESSAGES/CK.po (1)
17-27
: 🧹 Nitpick (assertive)Add missing Plural-Forms header
For consistency with other English locale files, include:"Plural-Forms: nplurals=2; plural=(n != 1)\n"⛔ Skipped due to learnings
Learnt from: PPsyrius PR: vacanza/holidays#2537 File: holidays/locale/sv_FI/LC_MESSAGES/AX.po:17-27 Timestamp: 2025-05-10T04:34:02.406Z Learning: The `Plural-Forms` header isn't used in .po file generation for the holidays project and doesn't need to be manually added to localization files.
< 8000 /div>Learnt from: PPsyrius PR: vacanza/holidays#2437 File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:0-0 Timestamp: 2025-04-06T14:52:35.679Z Learning: When suggesting changes to .po files in the holidays project, follow the standardized header format: - Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #" - Non-default language format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #" For example, for Bhutan in Dzongkha, use "# Bhutan holidays dz_BT localization. #"
holidays/locale/en_US/LC_MESSAGES/CK.po (1)
17-27
: 🧹 Nitpick (assertive)Add Plural-Forms header
To mirror standard PO headers, prepend:"Plural-Forms: nplurals=2; plural=(n != 1)\n"⛔ Skipped due to learnings
Learnt from: PPsyrius PR: vacanza/holidays#2537 File: holidays/locale/sv_FI/LC_MESSAGES/AX.po:17-27 Timestamp: 2025-05-10T04:34:02.406Z Learning: The `Plural-Forms` header isn't used in .po file generation for the holidays project and doesn't need to be manually added to localization files.
Learnt from: PPsyrius PR: vacanza/holidays#2437 File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:0-0 Timestamp: 2025-04-06T14:52:35.679Z Learning: When suggesting changes to .po files in the holidays project, follow the standardized header format: - Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #" - Non-default language format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #" For example, for Bhutan in Dzongkha, use "# Bhutan holidays dz_BT localization. #"
Learnt from: PPsyrius PR: vacanza/holidays#2437 File: holidays/locale/dz_BT/LC_MESSAGES/BT.po:12-12 Timestamp: 2025-04-06T14:44:59.785Z Learning: Use standardized .po file headers for localization files in the holidays project: - Default language format: "# [COUNTRY-NAME-NORMAL] holidays. #" - Other languages format: "# [COUNTRY-NAME-NORMAL] holidays [LANGUAGE-CODE] localization. #"
tests/countries/test_cook_islands.py (7)
19-26
: Excellent test class setup following established patterns.The class properly extends CommonCountryTests and TestCase, sets up a comprehensive year range (1999-2049), and efficiently creates subdivision holiday instances for testing. This follows the library's testing conventions perfectly.
28-29
: Proper alias testing implementation.Good verification that the CK and COK alias classes are properly configured as subclasses of CookIslands.
31-32
: Correct validation of start year boundary.Properly tests that no holidays exist before the 1999 start year, ensuring the implementation respects the Public Holiday Act 1999 effective date.
34-39
: Thorough New Year's Day testing with observed dates.Excellent coverage testing both the regular holiday and observed dates when New Year's falls on weekends. The specific years tested (2005, 2006, 2011, 2012, 2017) represent actual weekend occurrences.
125-139
: Well-structured subdivision holiday testing.The Aitutaki Gospel Day test properly verifies the holiday exists only before 2012, includes observed dates when falling on weekends, and confirms absence after 2011. This pattern is correctly applied across all subdivision tests.
266-301
: Comprehensive localization testing.Great coverage of both default (en_CK) and en_US localizations, including the important difference in "Ra o te Ui Ariki" vs "Day of the House of Ariki" naming. This ensures proper internationalization support.
89-101
:❓ Verification inconclusive
Verify House of Ariki Day date transitions are accurate.
The test correctly shows the holiday moving from June 6th (2011-2012) to first Friday of July (2013+), but please verify these specific dates against the actual legislation to ensure accuracy.
🏁 Script executed:
#!/bin/bash # Verify the House of Ariki Day date calculations for the test years echo "Verifying House of Ariki Day dates..." echo "2011-06-06 should be Monday (June 6th)" echo "2012-06-06 should be Wednesday (June 6th)" echo "2020-07-03 should be Friday (1st Friday of July)" echo "2021-07-02 should be Friday (1st Friday of July)" echo "2022-07-01 should be Friday (1st Friday of July)" echo "2023-07-07 should be Friday (1st Friday of July)" echo "2024-07-05 should be Friday (1st Friday of July)" echo "2025-07-04 should be Friday (1st Friday of July)" # Check if these are actually the first Fridays of July for the given years python3 -c " import datetime years = [2020, 2021, 2022, 2023, 2024, 2025] for year in years: # Find first Friday of July first_july = datetime.date(year, 7, 1) days_to_friday = (4 - first_july.weekday()) % 7 first_friday = first_july + datetime.timedelta(days=days_to_friday) print(f'{year} first Friday of July: {first_friday}') "Length of output: 1494
Verify House of Ariki Day dates against legislation
I’ve confirmed that the first Fridays of July for 2020–2025 in your tests all match the actual dates:
- 2020-07-03
- 2021-07-02
- 2022-07-01
- 2023-07-07
- 2024-07-05
- 2025-07-04
Please also:
- Cross-check that June 6, 2011 and June 6, 2012 were the official observance dates per Cook Islands holiday legislation.
- (Optional) Add test coverage for the transition years 2013–2019 to ensure there are no gaps.
holidays/countries/cook_islands.py (6)
23-36
: Excellent documentation with authoritative references.Outstanding use of official Cook Islands legal sources and government documents. The comprehensive reference list provides solid legal backing for the holiday implementations.
38-66
: Well-structured country metadata configuration.Proper setup of country code, languages, start year, and subdivisions. The subdivision aliases provide user-friendly access while maintaining the official codes.
68-72
: Correct initialization pattern with appropriate observed rule.Good practice explicitly initializing parent classes and setting a sensible default observed rule for weekend holidays.
76-79
: Correct observed holiday handling for New Year's period.Proper use of SAT_SUN_TO_NEXT_MON_TUE rule for both New Year's Day and Day after New Year's Day, which can create Tuesday observed dates when both fall on weekends.
119-162
: Consistent subdivision holiday implementation.All subdivision gospel day methods follow the same pattern - checking year < 2012 and applying observed rules. This matches the historical context where these were consolidated after 2011.
165-170
: Standard alias class implementation.Simple pass-through classes for alternative country codes follow the established library pattern.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2582 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 225 226 +1
Lines 14362 14409 +47
Branches 2017 2020 +3
=========================================
+ Hits 14362 14409 +47 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (7)
tests/countries/test_cook_islands.py (3)
63-63
: Missing range check for movable holiday.Following the project pattern for movable holidays, you should include a range check after the specific date assertions.
Apply this fix:
) + self.assertHolidayName(name, range(2000, 2050))
76-76
: 🧹 Nitpick (assertive)Missing range check for Easter Monday.
Similar to Good Friday, Easter Monday should include a range check after the specific dates.
Add the range check:
) + self.assertHolidayName(name, range(2000, 2050))
88-88
: 🧹 Nitpick (assertive)Missing range check for Sovereign's Birthday.
Add a range check to ensure this holiday exists across all test years.
Apply this fix:
) + self.assertHolidayName(name, range(2000, 2050))
holidays/countries/cook_islands.py (4)
28-35
: Archive non-Wikipedia references as suggested.The non-Wikipedia links should be archived using Internet Archive to prepare for future link stability requirements.
43-44
: Consider start_year alignment with Act implementation.Since the Public Holidays Act 1999 was implemented on December 17, 1999, consider whether the start_year should be 2000 instead of 1999 for practical purposes.
97-97
: Remove unnecessary parentheses.There are unnecessary parentheses around the method call that should be removed for cleaner code.
Apply this fix:
- (self._add_holiday_1st_fri_of_jul(name)) + self._add_holiday_1st_fri_of_jul(name)
120-162
: Inconsistent conditional operators in subdivision methods.The subdivision methods use both
<= 2011
and< 2012
conditions. For consistency, consider standardizing to one approach.Consider using
<= 2011
consistently:- if self._year < 2012: + if self._year <= 2011:
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/cook_islands.py
(1 hunks)tests/countries/test_cook_islands.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_cook_islands.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_cook_islands.py (2)
tests/common.py (8)
TestCase
(28-338)CommonCountryTests
(356-374)assertAliase 8000 s
(121-130)assertNoHolidays
(292-294)assertHolidayName
(195-199)assertNoNonObservedHoliday
(248-250)assertNoHolidayName
(273-275)assertLocalizedHolidays
(327-338)holidays/countries/cook_islands.py (3)
CookIslands
(23-162)CK
(165-166)COK
(169-170)
🔇 Additional comments (5)
tests/countries/test_cook_islands.py (2)
22-23
: Test setup looks good!The year range 2000-2049 is appropriate and the setup properly initializes both regular and non-observed holiday instances for comprehensive testing.
267-302
: Solid localization testing!The localization tests properly verify both the default en_CK locale and en_US locale, including the correct translation of "Ra o te Ui Ariki" to "Day of the House of Ariki" in US English. This ensures proper internationalization support.
holidays/countries/cook_islands.py (3)
23-66
: Well-structured class definition!The class properly inherits from the required base classes and has comprehensive metadata including country code, languages, subdivisions, and aliases. The subdivision definitions with their full names in comments are particularly helpful.
74-117
: Comprehensive national holiday implementation!The holiday implementation covers all the major Cook Islands holidays with proper observed date handling. The logic for Ra o te Ui Ariki changing from a fixed date to first Friday of July is correctly implemented, and the use of appropriate observed rules for different holidays shows good attention to detail.
165-170
: Clean alias implementation!The CK and COK alias classes provide the standard alternative country codes in a clean, simple manner.
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/countries/test_cook_islands.py (1)
163-177
: Same off-by-one error affects all subdivision gospel day tests.All subdivision gospel day tests have the same issue - they should include 2011 in the positive test range since the holidays existed until 2011 inclusive.
Apply the same fix to change
range(2000, 2011)
torange(2000, 2012)
in:
- Atiu Gospel Day (line 166)
- Mangaia Gospel Day (line 181)
- Manihiki Gospel Day (line 197)
- Mitiaro Gospel Day (line 213)
- Palmerston Gospel Day (line 228)
- Penrhyn Gospel Day (line 243)
- Pukapuka Gospel Day (line 259)
- Rakahanga Gospel Day (line 275)
Also applies to: 178-193, 194-209, 210-224, 225-239, 240-255, 256-271, 272-286
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/countries/test_cook_islands.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_cook_islands.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_cook_islands.py (2)
tests/common.py (8)
TestCase
(28-338)CommonCountryTests
(356-374)assertAliases
(121-130)assertNoHolidays
(292-294)assertHolidayName
(195-199)assertNoNonObservedHoliday
(248-250)assertNoHolidayName
(273-275)assertLocalizedHolidays
(327-338)holidays/countries/cook_islands.py (3)
CookIslands
(23-168)CK
(171-172)COK
(175-176)
🔇 Additional comments (4)
tests/countries/test_cook_islands.py (4)
13-26
: Test setup looks solid.The imports, class structure, and setup follow the established patterns correctly. Good use of subdivision holiday objects for comprehensive testing.
28-32
: Basic functionality tests are well-structured.Proper testing of aliases and boundary conditions (no holidays before 2000).
92-105
: House of Ariki Day test correctly handles the complex logic.The test properly covers the transition from fixed date (2012-06-06) to first Friday pattern (2013+), and includes the comprehensive range check. This addresses the concerns from previous reviews.
288-323
: Localization tests are comprehensive and correct.Good coverage of both default (en_CK) and en_US locales, properly testing the key difference in House of Ariki Day naming.
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/countries/test_cook_islands.py (1)
168-179
: 🧹 Nitpick (assertive)Penrhyn Gospel Day test could follow standard sequence pattern better.
While the test logic is correct, consider reordering to match the standard sequence used elsewhere: regular holiday assertions first, then observed holiday assertions.
Consider this reordering to match the standard pattern:
def test_penrhyn_gospel_day(self): name = "Penrhyn Gospel Day" self.assertHolidayName(name, (f"{year}-03-13" for year in range(2000, 2012))) + self.assertNoHolidayName(name, range(2012, 2050)) obs_dt = ( "2004-03-15", "2005-03-14", "2010-03-15", "2011-03-14", ) self.assertHolidayName(f"{name} (observed)", obs_dt) - self.assertNoHolidayName(name, range(2012, 2050)) self.assertNoHolidayName(f"{name} (observed)", range(2012, 2050))🧰 Tools
🪛 Pylint (3.3.7)
[convention] 168-168: Missing function or method docstring
(C0116)
holidays/countries/cook_islands.py (1)
143-148
: 🧹 Nitpick (assertive)Add docstrings to alias classes.
The alias classes are missing docstrings, which static analysis flags as an issue.
Add brief docstrings to address the static analysis warnings:
class CK(CookIslands): + """Alias for CookIslands.""" pass class COK(CookIslands): + """Alias for CookIslands.""" pass🧰 Tools
🪛 Pylint (3.3.7)
[convention] 143-143: Missing class docstring
(C0115)
[convention] 147-147: Missing class docstring
(C0115)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/cook_islands.py
(1 hunks)tests/countries/test_cook_islands.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_cook_islands.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.
🪛 Pylint (3.3.7)
tests/countries/test_cook_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 'TestCookIslands.setUpClass' method
(W0221)
[convention] 25-25: Missing function or method docstring
(C0116)
[convention] 28-28: Missing function or method docstring
(C0116)
[convention] 31-31: Missing function or method docstring
(C0116)
[convention] 44-44: Missing function or method docstring
(C0116)
[convention] 57-57: Missing function or method docstring
(C0116)
[convention] 60-60: Missing function or method docstring
(C0116)
[convention] 74-74: Missing function or method docstring
(C0116)
[convention] 88-88: Missing function or method docstring
(C0116)
[convention] 101-101: Missing function or method docstring
(C0116)
[convention] 116-116: Missing function or method docstring
(C0116)
[convention] 129-129: Missing function or method docstring
(C0116)
[convention] 142-142: Missing function or method docstring
(C0116)
[convention] 155-155: 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] 196-196: Missing function or method docstring
(C0116)
[convention] 208-208: Missing function or method docstring
(C0116)
[convention] 220-220: Missing function or method docstring
(C0116)
[convention] 232-232: Missing function or method docstring
(C0116)
[convention] 244-244: Missing function or method docstring
(C0116)
[convention] 256-256: Missing function or method docstring
(C0116)
[convention] 268-268: Missing function or method docstring
(C0116)
[convention] 280-280: Missing function or method docstring
(C0116)
[convention] 292-292: Missing function or method docstring
(C0116)
[convention] 304-304: Missing function or method docstring
(C0116)
[convention] 322-322: Missing function or method docstring
(C0116)
[refactor] 19-19: Too many public methods (27/20)
(R0904)
holidays/countries/cook_islands.py
[convention] 29-29: Line too long (207/100)
(C0301)
[convention] 30-30: Line too long (194/100)
(C0301)
[convention] 31-31: Line too long (214/100)
(C0301)
[convention] 32-32: Line too long (217/100)
(C0301)
[convention] 33-33: Line too long (112/100)
(C0301)
[convention] 34-34: Line too long (249/100)
(C0301)
[convention] 35-35: Line too long (201/100)
(C0301)
[convention] 36-36: Line too long (123/100)
(C0301)
[convention] 37-37: Line too long (114/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 143-143: Missing class docstring
(C0115)
[convention] 147-147: Missing class docstring
(C0115)
🔇 Additional comments (7)
tests/countries/test_cook_islands.py (4)
19-26
: Test class structure looks solid.The test class properly extends the required base classes and correctly sets up the years range to match the implementation's start_year. The country aliases test is also properly implemented.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestCookIslands.setUpClass' method
(W0221)
[convention] 25-25: Missing function or method docstring
(C0116)
[refactor] 19-19: Too many public methods (27/20)
(R0904)
31-42
: New Year's Day test follows correct pattern.Good implementation testing both the holiday dates across the full range and the observed dates with proper non-observed checks.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 31-31: Missing function or method docstring
(C0116)
101-114
: House of Ariki Day test correctly implements year-based logic.The test properly validates that the holiday starts in 2012, with correct range checks for both presence and absence periods. The implementation correctly reflects the legislative change from June 6th to first Friday of July starting in 2013.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 101-101: Missing function or method docstring
(C0116)
304-340
: Localization tests verify proper holiday name translations.The tests correctly verify that holiday names are properly localized in both default and en_US locales, including the different translation for "Ra o te Ui Ariki" vs "Day of the House of Ariki".
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 304-304: Missing function or method docstring
(C0116)
[convention] 322-322: Missing function or method docstring
(C0116)
holidays/countries/cook_islands.py (3)
24-46
: Class metadata and configuration look solid.The class properly inherits from the required base classes, has comprehensive documentation with archived references, and correctly sets up the country metadata including the start year of 2000.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Line too long (207/100)
(C0301)
[convention] 30-30: Line too long (194/100)
(C0301)
[convention] 31-31: Line too long (214/100)
(C0301)
[convention] 32-32: Line too long (217/100)
(C0301)
[convention] 33-33: Line too long (112/100)
(C0301)
[convention] 34-34: Line too long (249/100)
(C0301)
[convention] 35-35: Line too long (201/100)
(C0301)
[convention] 36-36: Line too long (123/100)
(C0301)
[convention] 37-37: Line too long (114/100)
(C0301)
79-85
: House of Ariki Day logic correctly implements legislative changes.The conditional logic properly implements the holiday starting in 2012, moving from June 6th to first Friday of July in 2013 as per the legislative amendments. The implementation matches the test expectations.
105-140
: Gospel Day holidays properly implement historical context.The implementation correctly limits these subdivision-specific holidays to years up to 2011, with proper observed holiday rules applied. The special case for Mauke Gospel Day in 2005 and 2011 is correctly handled.
Signed-off-by: tr33k <devarajkaliyamoorthy@gmail.com>
Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 comments (1)
README.md (1)
437-443
: Cook Islands table entry looks correct
The “Cook Islands” (CK) row is inserted in alphabetical order between Congo (CG) and Costa Rica (CR). The absence of subdivisions and additional categories aligns with design, and the default languageen_CK
is properly highlighted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last fixes.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Devaraj K <devarajkaliyamoorthy@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🇨🇰
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done, LGTM 👍
@tr33k could you take care of your GitHub sponsors account for the upcoming sponsorship payment? |
Proposed change
Add Cook Islands Holidays.
Resolves: #2427
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green