Add Turks and Caicos Islands holidays #2483
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughSupport for Turks and Caicos Islands holidays was added, including a new country module, localization files, registry updates, documentation, and a comprehensive test suite. Holiday logic, aliases, and translations were implemented, and a data snapshot was provided for validation. Changes
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (1)holidays/registry.py (1)🧬 Code Graph Analysis (1)holidays/countries/__init__.py (1)
🔇 Additional comments (5)
✨ 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: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
AUTHORS.md(1 hunks)README.md(1 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/turks_and_caicos_islands.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/TC.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_turks_and_caicos_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turks_and_caicos_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
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.
🧬 Code Graph Analysis (2)
holidays/countries/__init__.py (1)
holidays/countries/turks_and_caicos_islands.py (3)
TurksAndCaicosIslands(20-73)TC(76-77)TCA(80-81)
holidays/countries/turks_and_caicos_islands.py (3)
holidays/holiday_base.py (1)
HolidayBase(57-1275)holidays/groups/christian.py (5)
ChristianHolidays(22-463)_add_good_friday(308-317)_add_easter_monday(259-268)_add_christmas_day(208-216)_add_christmas_day_two(218-226)holidays/groups/international.py (2)
InternationalHolidays(18-220)_add_new_years_day(126-134)
🪛 Ruff (0.8.2)
tests/countries/test_turks_and_caicos_islands.py
75-75: No newline at end of file
Add trailing newline
(W292)
holidays/countries/turks_and_caicos_islands.py
15-15: holidays.constants.JAN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAR imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAY imported but unused
Remove unused import
(F401)
15-15: holidays.constants.JUN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.AUG imported but unused
Remove unused import
(F401)
15-15: holidays.constants.SEP imported but unused
Remove unused import
(F401)
15-15: holidays.constants.OCT imported but unused
Remove unused import
(F401)
15-15: holidays.constants.NOV imported but unused
Remove unused import
(F401)
15-15: holidays.constants.DEC imported but unused
Remove unused import
(F401)
22-22: Blank line contains whitespace
Remove whitespace from blank line
(W293)
81-81: No newline at end of file
Add trailing newline
(W292)
🔇 Additional comments (11)
AUTHORS.md (1)
135-135: Approve adding author. The new author "Shalom Donga" is correctly inserted alphabetically between "Serhii Murza" and "Shaurya Uppal."README.md (1)
1223-1228: Approve addition to Supported Countries table. The new row for Turks and Caicos Islands with codeTCand defaulten_USis formatted consistently and placed correctly between Turkey and Tuvalu.holidays/countries/__init__.py (1)
169-169: Approve import addition. TheTurksAndCaicosIslands, TC, TCAimport is correctly inserted afterTurkeyand beforeTuvalu, maintaining alphabetical order and consistency with existing patterns.holidays/locale/en_US/LC_MESSAGES/TC.po (1)
31-76: Approve holiday translations. Allmsgidentries accurately match the holiday names defined inTurksAndCaicosIslandsand themsgstrtranslations are correct for English.tests/countries/test_turks_and_caicos_islands.py (4)
19-23: Correctly implementing test class with CommonCountryTestsYour test class is properly structured with CommonCountryTests as the base class, which aligns well with the project's testing patterns. Using
setUpClassinstead ofsetUpis appropriate here since you're setting up once for all test methods.
24-26: LGTM: Country aliases test is completeGood verification of both country code aliases (TC and TCA) mapping to the main class.
27-43: Complete coverage of 2023 holidaysYour test effectively captures all the Turks and Caicos Islands holidays for 2023, including both fixed-date holidays and movable ones like Easter-related dates and various Monday holidays. The comments for each date are helpful for understanding what's being tested.
44-60: Great verification of 2025 holidaysTesting an additional year (2025) is a good practice to verify that movable holiday calculations work correctly across different years. All expected holidays are covered with proper dates.
holidays/countries/turks_and_caicos_islands.py (3)
29-37: Correct class initializationThe class initialization follows the project's patterns, properly setting up the country code, default language, and supported languages. The constructor also correctly initializes all parent classes.
38-74: Complete implementation of public holidaysYour implementation of
_populate_public_holidaysis thorough and follows the project's patterns. All holidays mentioned in the PR description are included with proper translations. You've correctly handled both fixed-date holidays and movable holidays using the appropriate helper methods.
15-15: 🧹 Nitpick (assertive)Remove unused month constant imports
Several month constants are imported but not used in the implementation. This happens because you're using the helper methods that don't require direct use of these constants.
-from holidays.constants import JAN, MAR, MAY, JUN, AUG, SEP, OCT, NOV, DEC⛔ 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.🧰 Tools
🪛 Ruff (0.8.2)
15-15:
holidays.constants.JANimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MARimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MAYimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.JUNimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.AUGimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.SEPimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.OCTimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.NOVimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.DECimported but unusedRemove unused import
(F401)
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
holidays/countries/turks_and_caicos_islands.py (1)
81-82: 🧹 Nitpick (assertive)Add missing newline at end of file.
Add a newline at the end of the file to comply with Python coding standards.
class TCA(TurksAndCaicosIslands): pass +
♻️ Duplicate comments (3)
holidays/locale/en_US/LC_MESSAGES/TC.po (1)
17-20: Metadata fields properly updated.The PO header fields have been correctly updated with:
- Holidays version 0.71
- Current creation and revision dates
- Your name and email as translator
This addresses the previous review comment about updating the placeholder values.
holidays/countries/turks_and_caicos_islands.py (2)
20-27: Well-documented holiday class with sources.Good job including multiple reference sources for the holiday definitions. This adds credibility to your implementation and helps future maintainers verify or update the holiday definitions.
76-81: Country code aliases properly defined.The TC and TCA alias classes are correctly implemented as simple subclasses of the main TurksAndCaicosIslands class.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/countries/turks_and_caicos_islands.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/TC.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_turks_and_caicos_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/registry.py (1)
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.
tests/countries/test_turks_and_caicos_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to common test utilities.
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.
🪛 Ruff (0.8.2)
tests/countries/test_turks_and_caicos_islands.py
76-76: Blank line contains whitespace
Remove whitespace from blank line
(W293)
76-76: No newline at end of file
Add trailing newline
(W292)
holidays/countries/turks_and_caicos_islands.py
15-15: holidays.constants.JAN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAR imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAY imported but unused
Remove unused import
(F401)
15-15: holidays.constants.JUN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.AUG imported but unused
Remove unused import
(F401)
15-15: holidays.constants.SEP imported but unused
Remove unused import
(F401)
15-15: holidays.constants.OCT imported but unused
Remove unused import
(F401)
15-15: holidays.constants.NOV imported but unused
Remove unused import
(F401)
15-15: holidays.constants.DEC imported but unused
Remove unused import
(F401)
🔇 Additional comments (11)
holidays/registry.py (1)
177-177: Registry entry correctly formatted.The added entry for the Turks and Caicos Islands follows the correct pattern with the class name "TurksAndCaicosIslands" as the first element of the tuple, along with the proper country codes "TC" and "TCA".
holidays/locale/en_US/LC_MESSAGES/TC.po (1)
30-76: Holiday translations complete and accurate.All holiday names are properly defined with matching English translations. The included holidays align with those implemented in the country class.
tests/countries/test_turks_and_caicos_islands.py (5)
19-23: Test class structure follows project conventions.The test class correctly inherits from
CommonCountryTestsandTestCase, and properly callssuper().setUpClass()with the appropriate parameters. This follows the project's test patterns.
24-25: Alias verification properly implemented.This test correctly verifies that the country code aliases (TC and TCA) properly reference the main TurksAndCaicosIslands class.
27-42: 2023 holidays correctly tested.The test includes all expected holidays for 2023 with accurate dates, including the renamed "King's Birthday" (formerly "Queen's Birthday").
44-59: 2025 holidays correctly tested.The test includes all expected holidays for 2025 with accurate dates, providing good coverage for future holiday calculations.
61-75: Localization test is comprehensive.The test verifies that all holiday names are correctly defined in the default locale, which is important for ensuring proper translation functionality.
holidays/countries/turks_and_caicos_islands.py (4)
29-31: Country metadata correctly defined.The country code, default language, and supported languages are properly specified, supporting both "en_TC" and "en_US" locales.
33-36: Class initialization properly implemented.The constructor correctly initializes the parent classes before calling the superclass constructor, which is essential for proper inheritance.
38-73: Holiday definitions comprehensive and properly documented.All the Turks and Caicos Islands holidays are correctly implemented with:
- Proper helper methods for date calculations
- Translated names using gettext
- Comments documenting each holiday
- Notes about historical name changes (Queen's Birthday → King's Birthday, Columbus Day → National Heritage Day)
This implementation covers all the official public holidays.
15-15: 🧹 Nitpick (assertive)Remove unused constant imports.
The month constants are imported but not directly used in the code. They're likely used internally by the helper methods, but should be removed to follow the project's code style.
-from holidays.constants import JAN, MAR, MAY, JUN, AUG, SEP, OCT, NOV, DEC +from holidays.holiday_base import HolidayBase +from holidays.groups import ChristianHolidays, InternationalHolidays⛔ 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.🧰 Tools
🪛 Ruff (0.8.2)
15-15:
holidays.constants.JANimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MARimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MAYimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.JUNimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.AUGimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.SEPimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.OCTimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.NOVimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.DECimported but unusedRemove unused import
(F401)
|
I have implemented the following changes:
|
|
@ShalomDee do you plan to finish your work or we can close this PR? |
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (2)
tests/countries/test_turks_and_caicos_islands.py (1)
156-157: 🧹 Nitpick (assertive)Add missing newline at end of file.
Add a newline character at the end of the file to follow standard coding practices.
("2025-12-25", "Christmas Day"), ("2025-12-26", "Boxing Day"), ) -157 +157 +holidays/countries/turks_and_caicos_islands.py (1)
85-86: 🧹 Nitpick (assertive)Add missing newline at end of file.
Add a newline character at the end of the file to follow standard coding practices.
class TCA(TurksAndCaicosIslands): pass -86 +86 +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/turks_and_caicos_islands.py(1 hunks)tests/countries/test_turks_and_caicos_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turks_and_caicos_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to 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.
🪛 Ruff (0.8.2)
tests/countries/test_turks_and_caicos_islands.py
27-27: Blank line contains whitespace
Remove whitespace from blank line
(W293)
32-32: Trailing whitespace
Remove trailing whitespace
(W291)
33-33: Line too long (101 > 99)
(E501)
34-34: Blank line contains whitespace
Remove whitespace from blank line
(W293)
37-37: Line too long (119 > 99)
(E501)
38-38: Blank line contains whitespace
Remove whitespace from blank line
(W293)
41-41: Line too long (119 > 99)
(E501)
42-42: Blank line contains whitespace
Remove whitespace from blank line
(W293)
57-57: Blank line contains whitespace
Remove whitespace from blank line
(W293)
59-59: Blank line contains whitespace
Remove whitespace from blank line
(W293)
64-64: Blank line contains whitespace
Remove whitespace from blank line
(W293)
69-69: Blank line contains whitespace
Remove whitespace from blank line
(W293)
73-73: Blank line contains whitespace
Remove whitespace from blank line
(W293)
75-75: Trailing whitespace
Remove trailing whitespace
(W291)
76-76: Line too long (103 > 99)
(E501)
77-77: Blank line contains whitespace
Remove whitespace from blank line
(W293)
82-82: Blank line contains whitespace
Remove whitespace from blank line
(W293)
86-86: Blank line contains whitespace
Remove whitespace from blank line
(W293)
88-88: Line too long (100 > 99)
(E501)
88-88: Trailing whitespace
Remove trailing whitespace
(W291)
89-89: Line too long (103 > 99)
(E501)
holidays/countries/turks_and_caicos_islands.py
15-15: holidays.constants.JAN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAR imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAY imported but unused
Remove unused import
(F401)
15-15: holidays.constants.JUN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.AUG imported but unused
Remove unused import
(F401)
15-15: holidays.constants.SEP imported but unused
Remove unused import
(F401)
15-15: holidays.constants.OCT imported but unused
Remove unused import
(F401)
15-15: holidays.constants.NOV imported but unused
Remove unused import
(F401)
15-15: holidays.constants.DEC imported but unused
Remove unused import
(F401)
🔇 Additional comments (6)
tests/countries/test_turks_and_caicos_islands.py (1)
1-156: Excellent test coverage for the Turks and Caicos Islands holidays.The test module provides comprehensive coverage of all fixed and movable holidays for the Turks and Caicos Islands, including proper handling of:
- Historical holiday name changes (JAGS McCartney Day, National Heritage Day)
- Sovereign transitions (Queen's Birthday to King's Birthday)
- Movable holidays based on Easter dates and weekday rules
- Default and English US localization
This follows the project's established patterns for holiday tests, as seen in other country test modules.
🧰 Tools
🪛 Ruff (0.8.2)
27-27: Blank line contains whitespace
Remove whitespace from blank line
(W293)
32-32: Trailing whitespace
Remove trailing whitespace
(W291)
33-33: Line too long (101 > 99)
(E501)
34-34: Blank line contains whitespace
Remove whitespace from blank line
(W293)
37-37: Line too long (119 > 99)
(E501)
38-38: Blank line contains whitespace
Remove whitespace from blank line
(W293)
41-41: Line too long (119 > 99)
(E501)
42-42: Blank line contains whitespace
Remove whitespace from blank line
(W293)
57-57: Blank line contains whitespace
Remove whitespace from blank line
(W293)
59-59: Blank line contains whitespace
Remove whitespace from blank line
(W293)
64-64: Blank line contains whitespace
Remove whitespace from blank line
(W293)
69-69: Blank line contains whitespace
Remove whitespace from blank line
(W293)
73-73: Blank line contains whitespace
Remove whitespace from blank line
(W293)
75-75: Trailing whitespace
Remove trailing whitespace
(W291)
76-76: Line too long (103 > 99)
(E501)
77-77: Blank line contains whitespace
Remove whitespace from blank line
(W293)
82-82: Blank line contains whitespace
Remove whitespace from blank line
(W293)
86-86: Blank line contains whitespace
Remove whitespace from blank line
(W293)
88-88: Line too long (100 > 99)
(E501)
88-88: Trailing whitespace
Remove trailing whitespace
(W291)
89-89: Line too long (103 > 99)
(E501)
holidays/countries/turks_and_caicos_islands.py (5)
54-55: Good historical context for JAGS McCartney Day.The implementation correctly handles the transition from "National Heroes Day" to "JAGS McCartney Day" in 2020, which matches historical records. The comment provides clear context about this change.
58-59: Good handling of sovereign transition.The code correctly implements the transition from "Queen's Birthday" to "King's Birthday" in 2023 following the accession of King Charles III. The comment provides clear context about this change.
20-33: Well-structured holiday class with clear documentation.The class is well-documented with multiple references to verify the holiday data. The implementation includes:
- Proper start year (1963) when the Turks and Caicos Islands separated from Jamaica
- Default and supported languages correctly set
- Good historical context in comments
This follows the project's established patterns for holiday implementations.
40-77: Well-implemented holiday definitions with proper localization.The implementation correctly:
- Uses helper methods for commonly defined holiday patterns
- Wraps all strings in translation functions for localization
- Includes clear comments for each holiday
- Handles name changes based on historical events
- Implements both fixed date and movable holidays
This follows the established patterns in the project and should work correctly.
15-15: 🧹 Nitpick (assertive)Remove unused month constants.
The month constants are imported but not directly used in the code. They're implicitly used through the helper methods, but not explicitly referenced.
-from holidays.constants import JAN, MAR, MAY, JUN, AUG, SEP, OCT, NOV, DEC +# Month constants not needed as they're used implicitly in helper methods⛔ 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.🧰 Tools
🪛 Ruff (0.8.2)
15-15:
holidays.constants.JANimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MARimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MAYimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.JUNimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.AUGimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.SEPimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.OCTimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.NOVimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.DECimported but unusedRemove unused import
(F401)
Yes, I do. I've just updated the tests. It should be okay now. |
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
tests/countries/test_turks_and_caicos_islands.py (1)
106-172: 🧹 Nitpick (assertive)Holiday date tests and localization tests look complete.
The year-specific tests for 2023 and 2025 are comprehensive, covering all expected holidays with correct dates. The localization tests for default language and en_US are correctly implemented.
However, there's a missing newline at the end of the file. Add one to comply with the project's formatting standards.
("2025-12-25", "Christmas Day"), ("2025-12-26", "Boxing Day"), ) +holidays/countries/turks_and_caicos_islands.py (1)
80-86: 🧹 Nitpick (assertive)Country code alias classes properly defined.
The TC and TCA alias classes are correctly implemented as simple subclasses of the main TurksAndCaicosIslands class.
Ensure there's a newline at the end of the file as per project conventions.
class TCA(TurksAndCaicosIslands): pass +
♻️ Duplicate comments (4)
tests/countries/test_turks_and_caicos_islands.py (4)
52-73: 🧹 Nitpick (assertive)Clean up whitespace in blank lines of jags_mccartney_day test.
There's whitespace in blank lines 66, 68, and 73 that should be removed.
def test_jags_mccartney_day(self): for year in range(2020, 2026): if year == 2020: dt = date(2020, 5, 25) elif year == 2021: dt = date(2021, 5, 31) elif year == 2022: dt = date(2022, 5, 30) elif year == 2023: dt = date(2023, 5, 29) elif year == 2024: dt = date(2024, 5, 27) else: # 2025 dt = date(2025, 5, 26) - + self.assertIn(dt, self.holidays) - + if year >= 2020: self.assertEqual(self.holidays[dt], "JAGS McCartney Day") else: self.assertEqual(self.holidays[dt], "National Heroes Day")🧰 Tools
🪛 Ruff (0.8.2)
66-66: Blank line contains whitespace
Remove whitespace from blank line
(W293)
68-68: Blank line contains whitespace
Remove whitespace from blank line
(W293)
74-82: 🧹 Nitpick (assertive)Remove trailing whitespace in sovereign_birthday test.
Line 78 contains trailing whitespace that should be removed.
def test_sovereign_birthday(self): holidays_2022 = TurksAndCaicosIslands(years=2022) self.assertIn(date(2022, 6, 13), holidays_2022) self.assertEqual(holidays_2022[date(2022, 6, 13)], "Queen's Birthday") - + holidays_2023 = TurksAndCaicosIslands(years=2023) self.assertIn(date(2023, 6, 12), holidays_2023) self.assertEqual(holidays_2023[date(2023, 6, 12)], "King's Birthday")🧰 Tools
🪛 Ruff (0.8.2)
78-78: Blank line contains whitespace
Remove whitespace from blank line
(W293)
90-98: 🧹 Nitpick (assertive)Remove trailing whitespace in national_heritage_day test.
Line 94 contains trailing whitespace that should be removed.
def test_national_heritage_day(self): holidays_2022 = TurksAndCaicosIslands(years=2022) self.assertIn(date(2022, 10, 10), holidays_2022) self.assertEqual(holidays_2022[date(2022, 10, 10)], "National Heritage Day") - + holidays_2025 = TurksAndCaicosIslands(years=2025) self.assertIn(date(2025, 10, 13), holidays_2025) self.assertEqual(holidays_2025[date(2025, 10, 13)], "National Heritage Day")🧰 Tools
🪛 Ruff (0.8.2)
94-94: Blank line contains whitespace
Remove whitespace from blank line
(W293)
45-50: 🧹 Nitpick (assertive)Fix trailing whitespace and line length in easter_monday test.
Same issues with trailing whitespace and line length in this test method.
def test_easter_monday(self): self.assertHolidayName( - "Easter Monday", - (f"{year}-{month}-{day}" for year, month, day in - [(2020, 4, 13), (2021, 4, 5), (2022, 4, 18), (2023, 4, 10), (2024, 4, 1), (2025, 4, 21)]) + "Easter Monday", + (f"{year}-{month}-{day}" for year, month, day in + [(2020, 4, 13), (2021, 4, 5), (2022, 4, 18), (2023, 4, 10), + (2024, 4, 1), (2025, 4, 21)]) )🧰 Tools
🪛 Ruff (0.8.2)
47-47: Trailing whitespace
Remove trailing whitespace
(W291)
48-48: Trailing whitespace
Remove trailing whitespace
(W291)
49-49: Line too long (102 > 99)
(E501)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/turks_and_caicos_islands.py(1 hunks)tests/countries/test_turks_and_caicos_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turks_and_caicos_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to 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 (1)
holidays/countries/turks_and_caicos_islands.py (3)
holidays/holiday_base.py (1)
HolidayBase(57-1301)holidays/groups/christian.py (5)
ChristianHolidays(22-463)_add_good_friday(308-317)_add_easter_monday(259-268)_add_christmas_day(208-216)_add_christmas_day_two(218-226)holidays/groups/international.py (2)
InternationalHolidays(18-220)_add_new_years_day(126-134)
🪛 Ruff (0.8.2)
holidays/countries/turks_and_caicos_islands.py
15-15: holidays.constants.JAN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAR imported but unused
Remove unused import
(F401)
15-15: holidays.constants.MAY imported but unused
Remove unused import
(F401)
15-15: holidays.constants.JUN imported but unused
Remove unused import
(F401)
15-15: holidays.constants.AUG imported but unused
Remove unused import
(F401)
15-15: holidays.constants.SEP imported but unused
Remove unused import
(F401)
15-15: holidays.constants.OCT imported but unused
Remove unused import
(F401)
15-15: holidays.constants.NOV imported but unused
Remove unused import
(F401)
15-15: holidays.constants.DEC imported but unused
Remove unused import
(F401)
tests/countries/test_turks_and_caicos_islands.py
33-33: Trailing whitespace
Remove trailing whitespace
(W291)
34-34: Trailing whitespace
Remove trailing whitespace
(W291)
40-40: Trailing whitespace
Remove trailing whitespace
(W291)
41-41: Trailing whitespace
Remove trailing whitespace
(W291)
42-42: Line too long (102 > 99)
(E501)
47-47: Trailing whitespace
Remove trailing whitespace
(W291)
48-48: Trailing whitespace
Remove trailing whitespace
(W291)
49-49: Line too long (102 > 99)
(E501)
66-66: Blank line contains whitespace
Remove whitespace from blank line
(W293)
68-68: Blank line contains whitespace
Remove whitespace from blank line
(W293)
78-78: Blank line contains whitespace
Remove whitespace from blank line
(W293)
85-85: Trailing whitespace
Remove trailing whitespace
(W291)
86-86: Trailing whitespace
Remove trailing whitespace
(W291)
94-94: Blank line contains whitespace
Remove whitespace from blank line
(W293)
101-101: Trailing whitespace
Remove trailing whitespace
(W291)
102-102: Trailing whitespace
Remove trailing whitespace
(W291)
🔇 Additional comments (13)
tests/countries/test_turks_and_caicos_islands.py (2)
1-18: Imports and setup look good.The imports properly include the necessary modules and classes to test the Turks and Caicos Islands holidays. You've correctly imported TestCase, CommonCountryTests, and the holiday implementation classes.
20-27: Test class setup follows project conventions.The TestTC class correctly inherits from CommonCountryTests and TestCase, with proper setup using the years range. The test_country_aliases and test_no_checks methods ensure basic functionality is validated.
holidays/countries/turks_and_caicos_islands.py (11)
1-12: Header looks good with proper copyright notices.The file header correctly includes the standard copyright information and license notices that match the project's format.
20-27: Well-documented holiday class with proper references.Excellent job including multiple reference sources for the holiday definitions. This adds credibility to your implementation and helps future maintainers verify or update the holiday definitions.
29-33: Country metadata properly defined.The country code, default language, start year, and supported languages are correctly defined. Including the historical context about separation from Jamaica in 1962 is a good practice.
35-38: Proper initialization of parent classes.The class constructor correctly initializes the parent classes before calling the super initializer.
40-42: New Year's Day correctly implemented.The implementation correctly adds New Year's Day using the helper method from InternationalHolidays.
44-45: Commonwealth Day correctly implemented.The implementation adds Commonwealth Day on the second Monday of March.
47-51: Easter-related holidays correctly implemented.The implementation adds Good Friday and Easter Monday using the helper methods from ChristianHolidays.
53-55: Good historical context for JAGS McCartney Day.Excellent implementation with conditional naming based on the year. The comment provides helpful historical context about the name change from National Heroes Day.
57-59: Good historical handling of Sovereign's Birthday.The implementation correctly handles the change from Queen's Birthday to King's Birthday in 2023.
61-77: Remaining holidays implemented correctly.All other holidays (Emancipation Day, National Youth Day, National Heritage Day, National Day of Thanksgiving, Christmas Day, and Boxing Day) are correctly implemented using appropriate helper methods.
The comment for National Heritage Day provides good historical context about the previous name (Columbus Day).
13-18: 🧹 Nitpick (assertive)Unused constants imports.
You've imported month constants from holidays.constants that aren't directly used in the code. While they might be used indirectly by the helper methods, it's cleaner to remove unused imports.
from gettext import gettext as tr -from holidays.constants import JAN, MAR, MAY, JUN, AUG, SEP, OCT, NOV, DEC from holidays.holiday_base import HolidayBase from holidays.groups import ChristianHolidays, InternationalHolidays⛔ 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.🧰 Tools
🪛 Ruff (0.8.2)
15-15:
holidays.constants.JANimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MARimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.MAYimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.JUNimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.AUGimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.SEPimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.OCTimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.NOVimported but unusedRemove unused import
(F401)
15-15:
holidays.constants.DECimported but unusedRemove unused import
(F401)
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
tests/countries/test_turks_and_caicos_islands.py (1)
213-213: 🧹 Nitpick (assertive)Add newline at end of file.
The file is missing a newline at the end, which is a common convention and will prevent linter warnings.
) +
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/countries/test_turks_and_caicos_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turks_and_caicos_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to 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.
🪛 Ruff (0.8.2)
tests/countries/test_turks_and_caicos_islands.py
13-13: datetime.date imported but unused
Remove unused import: datetime.date
(F401)
67-67: Blank line contains whitespace
Remove whitespace from blank line
(W293)
73-73: Trailing whitespace
Remove trailing whitespace
(W291)
77-77: Blank line contains whitespace
Remove whitespace from blank line
(W293)
86-86: Blank line contains whitespace
Remove whitespace from blank line
(W293)
93-93: Blank line contains whitespace
Remove whitespace from blank line
(W293)
100-100: Blank line contains whitespace
Remove whitespace from blank line
(W293)
120-120: Blank line contains whitespace
Remove whitespace from blank line
(W293)
130-130: Blank line contains whitespace
Remove whitespace from blank line
(W293)
🔇 Additional comments (8)
tests/countries/test_turks_and_caicos_islands.py (8)
20-26: Well-structured test class that follows project conventions.The test class correctly inherits from
CommonCountryTestsandTestCase, and follows the same patterns as other country test files in the project. Good job usingsetUpClassfor efficient test setup.
28-30: Good implementation of no_checks test.Including this test confirms that the holiday implementation doesn't require additional validation beyond the standard checks.
31-40: Well-structured movable holiday tests.The tests for movable holidays like Commonwealth Day, Good Friday, Easter Monday, National Youth Day, and National Day of Thanksgiving are properly implemented. Using individual date strings is more readable than a generator expression and aligns with recommended patterns.
Also applies to: 42-51, 53-62, 106-115, 136-145
64-82: Comprehensive test for holiday with historical name change.The test for JAGS McCartney Day correctly verifies both the current name (from 2020) and the historical name (before 2020), including proper assertions that the old name isn't used after 2020 and vice versa.
🧰 Tools
🪛 Ruff (0.8.2)
67-67: Blank line contains whitespace
Remove whitespace from blank line
(W293)
73-73: Trailing whitespace
Remove trailing whitespace
(W291)
77-77: Blank line contains whitespace
Remove whitespace from blank line
(W293)
83-105: Well-implemented sovereign birthday test.The test correctly handles the transition from Queen's Birthday to King's Birthday in 2023, with appropriate assertions for date ranges.
🧰 Tools
🪛 Ruff (0.8.2)
86-86: Blank line contains whitespace
Remove whitespace from blank line
(W293)
93-93: Blank line contains whitespace
Remove whitespace from blank line
(W293)
100-100: Blank line contains whitespace
Remove whitespace from blank line
(W293)
117-135: Good historical tracking for National Heritage Day.Similar to the other historical changes, this test correctly verifies the transition from Columbus Day to National Heritage Day in 2020.
🧰 Tools
🪛 Ruff (0.8.2)
120-120: Blank line contains whitespace
Remove whitespace from blank line
(W293)
130-130: Blank line contains whitespace
Remove whitespace from blank line
(W293)
147-179: Comprehensive year-specific holiday tests.The tests for specific years (2023 and 2025) verify all holidays expected for those years, providing a good cross-check against the individual holiday tests.
181-212: Good localization testing.Testing both default localization and explicit en_US localization ensures translation functionality works correctly.
|
@ShalomDee, , first of all, you need to:
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
holidays/countries/turks_and_caicos_islands.py(1 hunks)holidays/locale/en_TC/LC_MESSAGES/TC.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/TC.po(1 hunks)tests/countries/test_turks_and_caicos_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_turks_and_caicos_islands.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:31-49
Timestamp: 2025-04-05T04:50:40.752Z
Learning: For Turkmenistan holiday tests, use this class structure: `class TestTurkmenistan(CommonCountryTests, TestCase)` with imports `from unittest import TestCase`, `from holidays.countries import Turkmenistan, TM, TKM`, and `from tests.common import CommonCountryTests`. Ensure to call `super().setUp()` in the setUp method.
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:33:53.254Z
Learning: For Turkmenistan holiday tests, recommend using CommonCountryTests as the base class rather than unittest.TestCase to follow project conventions, be consistent with other country test files, and gain access to 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.
🔇 Additional comments (17)
holidays/locale/en_TC/LC_MESSAGES/TC.po (4)
47-54: Good historical context for JAGS McCartney Day.The comments explaining the historical name change from National Heroes Day to JAGS McCartney Day in 2020 are valuable for future maintainers.
71-74: Good historical context for National Heritage Day.Properly documenting the name change from Columbus Day to National Heritage Day in 2014 provides important historical context.
31-86: All necessary holiday translations are included.The file contains all the required holiday entries corresponding to the holidays defined in the Turks and Caicos Islands module, including both current and historical holiday names.
19-21:✅ Verification successful
Verify project version and dates in the PO header.
The file shows "Project-Id-Version: Holidays 0.72" and creation dates in 2025. These appear to be future dates which is unusual. According to previous comments, version 0.71 might be the correct current version.
🏁 Script executed:
#!/bin/bash # Check current version in other locale files grep -r "Project-Id-Version" --include="*.po" holidays/locale/ | head -5Length of output: 447
🏁 Script executed:
#!/bin/bash # Verify the authoritative project version grep -R "version" --include="setup.py" . grep -R "version" --include="pyproject.toml" . grep -R "__version__" -n holidays/__init__.py || trueLength of output: 336
Let’s fetch the actual
__version__fromholidays/version.pyto confirm the current release:#!/bin/bash grep -R "__version__" -n holidays/version.py sed -n '1,50p' holidays/version.py
Let’s confirm the canonical project version in
holidays/version.py:
🏁 Script executed:
#!/bin/bash grep -n "__version__" holidays/version.py sed -n '1,50p' holidays/version.pyLength of output: 653
No changes needed: PO header version and dates are correct
The
Project-Id-Version: Holidays 0.72inholidays/locale/en_TC/LC_MESSAGES/TC.pomatches the canonical__version__ = "0.72"inholidays/version.py, and the PO creation/revision dates reflect the current 2025-04-27 timestamp. Everything is up-to-date.holidays/locale/en_US/LC_MESSAGES/TC.po (3)
15-26: Well-formatted PO header with contributor information.The header includes the correct language (en_US), contributor information (Shalom Donga), and appropriate metadata. This addresses previous review comments about updating placeholder values.
28-75: Complete set of current holiday translations.All current holiday translations are properly defined. Since this is for English to English, the translations match the original text, which is expected.
76-83:✅ Verification successful
Empty translations for historical holiday names.
The entries for the older holiday names (National Heroes Day, Queen's Birthday) have empty translations. This is likely intentional to handle only current holiday names in the US locale.
Is this the intended behavior for historical holiday names? Other localization files might show similar patterns:
🏁 Script executed:
#!/bin/bash # Check how historical holiday names are handled in other locale files grep -r -A 1 "msgid \"National Heroes Day\"" --include="*.po" holidays/locale/Length of output: 847
Historical holiday names intentionally left untranslated in TC files
The empty msgstr entries for “National Heroes Day” and “Queen’s Birthday” in holidays/locale/en_US/LC_MESSAGES/TC.po match the pattern across other *TC.po files (e.g. en_TC/LC_MESSAGES/TC.po), ensuring the msgid is used as a fallback for legacy names. No changes required.
holidays/countries/turks_and_caicos_islands.py (6)
20-26: Well-documented holiday class with multiple sources.The docstring provides multiple reference sources for the holiday definitions which adds credibility to the implementation and helps future maintainers verify the holiday information.
28-32: Proper country metadata with historical context.The class correctly defines country code, default language, and supported languages. The comment about separation from Jamaica in 1962 provides useful historical context for the start_year setting.
34-37: Proper parent class initialization.The constructor correctly initializes all parent classes (ChristianHolidays, InternationalHolidays) before calling the super constructor.
52-54: Correctly implemented historical name change for JAGS McCartney Day.The code properly handles the name change from "National Heroes Day" to "JAGS McCartney Day" in 2020 with a clear comment.
56-58: Correctly implemented sovereign birthday name change.The code properly handles the change from "Queen's Birthday" to "King's Birthday" in 2023 with appropriate conditional logic.
78-84: Country code aliases properly defined.The TC and TCA alias classes are correctly implemented as simple subclasses of the main TurksAndCaicosIslands class.
tests/countries/test_turks_and_caicos_islands.py (4)
19-22: Test class properly inherits from CommonCountryTests.Following the project convention shown in the retrieved learnings, the test class properly inherits from CommonCountryTests and correctly initializes with the appropriate year range.
120-138: Well-structured test for National Heritage Day with thorough history validation.The test properly verifies the National Heritage Day holiday (formerly Columbus Day) across different years. It includes assertions for both the new and old names in appropriate year ranges.
Note: Make sure this test remains aligned with any changes you make to the
turks_and_caicos_islands.pyfile if you implement the year-based name change logic suggested earlier.
66-107: Comprehensive tests for historical name changes.The tests for JAGS McCartney Day and Sovereign Birthday thoroughly verify both the specific holiday dates and the name changes across appropriate year ranges, following best practices from the project.
185-216: Thorough localization testing.The tests verify holiday names in both the default locale (en_TC) and US English (en_US), ensuring the localization files are properly connected to the holiday definitions.
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
holidays/locale/en_TC/LC_MESSAGES/TC.po (1)
84-84: 🧹 Nitpick (assertive)Ensure newline at end of file.
Add a trailing newline to comply with POSIX text file conventions.holidays/locale/en_US/LC_MESSAGES/TC.po (1)
85-85: 🧹 Nitpick (assertive)Add missing newline at EOF.
Ensure there’s a blank line at the end of the file to meet file-formatting standards.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/locale/en_TC/LC_MESSAGES/TC.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/TC.po(1 hunks)
🔇 Additional comments (2)
holidays/locale/en_TC/LC_MESSAGES/TC.po (1)
17-17: Verify PO version matches project version.
The header’sProject-Id-Version: Holidays 0.72should align with the current dev/release version.Please confirm or update to the correct version (e.g., 0.73) before merging.
holidays/locale/en_US/LC_MESSAGES/TC.po (1)
17-19: Review PO header dates and version.
Please confirm thatProject-Id-Version: 0.72,POT-Creation-Date, andPO-Revision-Datereflect the latest commit and release cycle.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2483 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 228 229 +1
Lines 14533 14571 +38
Branches 2038 2039 +1
=========================================
+ Hits 14533 14571 +38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Here also is a discrepancy between the name in Ordinance and in the schedules. I think it's enough to keep JAGS. |
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (3)
holidays/locale/en_TC/LC_MESSAGES/TC.po (1)
84-84: 🧹 Nitpick (assertive)Ensure a trailing newline at end of file.
POSIX standards recommend that text files end with a newline.holidays/locale/en_US/LC_MESSAGES/TC.po (2)
78-85: 🧹 Nitpick (assertive)Clarify translator comments for alias entries.
Both alias entries ("National Heroes Day"and"Queen’s Birthday") repeat the main holiday comment. Consider updating them to explicitly note they are former holiday names:#. Alias: Former holiday name (National Heroes Day) until 2020. #. Alias: Former holiday name (Queen’s Birthday) until 2023.
85-85: 🧹 Nitpick (assertive)Ensure a trailing newline at end of file.
Adding a newline at EOF improves compatibility with POSIX tools.
♻️ Duplicate comments (2)
holidays/locale/en_TC/LC_MESSAGES/TC.po (1)
15-26: 🧹 Nitpick (assertive)Consider unifying generator metadata across locales.
The header here usesGenerated-By: Lingva 5.0.6without anX-Generatortag, whereas other locale files (e.g., en_US) include bothGenerated-ByandX-Generator. For consistency and easier maintenance, standardize the metadata fields across all locales.holidays/locale/en_US/LC_MESSAGES/TC.po (1)
17-28: 🧹 Nitpick (assertive)Align generator metadata with other locales.
This file usesGenerated-By: Lingua 4.15.0andX-Generator: Poedit 3.2.2, while the en_TC locale uses a differentGenerated-Bystring. Standardizing these tags across locales will help maintain consistency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/locale/en_TC/LC_MESSAGES/TC.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/TC.po(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/locale/en_US/LC_MESSAGES/TC.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.
🔇 Additional comments (4)
holidays/locale/en_TC/LC_MESSAGES/TC.po (3)
29-48: Primary holiday entries and placeholders look good.
All fixed and movable holiday entries include a descriptive comment, amsgid, and an emptymsgstrawaiting translation.
53-56: King’s Birthday entry is correctly formatted.
Primary holiday and its comment align with project conventions.
61-84: Remaining holiday entries are correctly formatted.
Entries from Emancipation Day through Boxing Day follow the expected.postructure.holidays/locale/en_US/LC_MESSAGES/TC.po (1)
30-77: Holiday translations are correctly applied.
All primary and historicalmsgidvalues have matchingmsgstrentries in US English.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
holidays/locale/en_US/LC_MESSAGES/TC.po (1)
89-89: 🧹 Nitpick (assertive)Add newline at end of file.
Ensure there’s a trailing newline for POSIX compliance.holidays/locale/en_TC/LC_MESSAGES/TC.po (1)
88-88: 🧹 Nitpick (assertive)Add newline at end of file.
Ensure a trailing newline is present for consistency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)holidays/locale/en_TC/LC_MESSAGES/TC.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/TC.po(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/locale/en_TC/LC_MESSAGES/TC.po (2)
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#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.").
🔇 Additional comments (7)
holidays/locale/en_US/LC_MESSAGES/TC.po (3)
13-14: Localization comment clarity.
The comment# Turks and Caicos Islands holidays en_US localization.clearly indicates this file’s purpose.
17-28: Header metadata is accurate.
Project-Id-Version, POT-Creation-Date, PO-Revision-Date, Last-Translator, and generator tags look correct and up-to-date.
30-88: Translation entries look good.
Allmsgid/msgstrpairs are correctly defined for US English; no fuzzy flags remain.holidays/locale/en_TC/LC_MESSAGES/TC.po (2)
17-23: Validate header version and language.
TheProject-Id-Version: Holidays 0.75andLanguage: en_TCsettings correctly reflect the new locale.
29-88: Untranslated entries initialized.
All holiday entries are present with emptymsgstrplaceholders, ready for localization.README.md (2)
108-108: Supported country count updated.
The count182correctly reflects the addition of Turks and Caicos Islands to the supported list.
1305-1310: Add Turks and Caicos Islands entry.
The<tr>for TC is placed alphabetically, with<strong>en_TC</strong>as the default anden_USsecondary.
|
|
@coderabbitai resolve |
✅ Actions performedComments resolved. |
Proposed change
This PR adds official public holiday support for the Turks and Caicos Islands (TC) to the
holidayspackage.Issue: Fixes #2435
Holidays included are based on:
The implementation covers fixed holidays and movable holidays like Easter-related dates and the King's Birthday.
Type of change
Checklist
make check, all checks and tests are green