-
-
Notifications
You must be signed in to change notification settings - Fork 539
Add Antigua and Barbuda holidays #2395
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
Summary by CodeRabbit
WalkthroughThis pull request introduces holiday support for Antigua and Barbuda, updating the README to reflect the new country code. It adds a module for holiday definitions specific to Antigua and Barbuda, updates import statements, and modifies the registry to recognize the new country. Additionally, a comprehensive set of unit tests is included to validate the holiday logic for Antigua and Barbuda across a specified range of years. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md
(2 hunks)holidays/countries/__init__.py
(1 hunks)holidays/countries/antigua_and_barbuda.py
(1 hunks)holidays/registry.py
(1 hunks)tests/countries/test_antigua_and_barbuda.py
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
holidays/countries/__init__.py (1)
holidays/countries/antigua_and_barbuda.py (3)
AntiguaAndBarbuda
(23-96)AG
(99-100)ATG
(103-104)
tests/countries/test_antigua_and_barbuda.py (1)
holidays/countries/antigua_and_barbuda.py (3)
AntiguaAndBarbuda
(23-96)AG
(99-100)ATG
(103-104)
🔇 Additional comments (28)
holidays/countries/__init__.py (1)
21-21
: LGTM: Import follows established pattern.The import statement for Antigua and Barbuda is correctly positioned alphabetically between Angola and Argentina, and includes the main class and its ISO code aliases.
README.md (2)
102-102
: Accurately updated country count.The total number of supported countries has been properly incremented from 161 to 162 to reflect the addition of Antigua and Barbuda.
186-191
: Entry correctly positioned and formatted.The Antigua and Barbuda table entry is properly placed alphabetically and follows the established table format. The country code AG matches the ISO 3166-1 alpha-2 code defined in the implementation.
holidays/registry.py (1)
29-29
: Registry entry correctly added.The Antigua and Barbuda entry is properly positioned alphabetically in the COUNTRIES dictionary and includes the correct class name and ISO code identifiers (AG, ATG).
tests/countries/test_antigua_and_barbuda.py (17)
1-12
: Standard header correctly implemented.The copyright and license header follows the project's established format.
14-16
: Imports are appropriate and complete.All necessary classes are imported from the appropriate modules.
18-25
: Test class setup is correct.The test class properly inherits from CommonCountryTests and TestCase, and the setup method correctly initializes the tests for years 1955-2050, matching the implementation's start_year.
26-39
: New Year's Day tests are comprehensive.Tests properly verify both the regular holiday and observed dates when January 1 falls on a Sunday.
40-51
: Good Friday tests are comprehensive.Tests correctly verify this moveable feast date across multiple years.
52-63
: Easter Monday tests are comprehensive.Tests correctly verify this moveable feast date across multiple years.
64-75
: Labour Day tests are comprehensive.Tests properly verify the first Monday in May across multiple years.
76-87
: Whit Monday tests are comprehensive.Tests correctly verify this moveable feast date across multiple years.
88-103
: Caribbean Community Day tests are correct.Tests properly verify that this holiday is only observed before 2005, as specified in the implementation.
104-115
: Carnival Monday tests are comprehensive.Tests properly verify the first Monday in August across multiple years.
116-128
: Carnival Tuesday tests are correct.Tests properly verify that this holiday is only observed after 2005, as specified in the implementation.
129-136
: Independence Day tests are comprehensive.Tests properly verify both the regular holiday and observed dates when November 1 falls on a weekend.
137-145
: National Heroes Day tests are correct.Tests properly verify that this holiday is only observed from 2005 to 2013, as specified in the implementation.
146-153
: Sir Vere Cornwall Bird Day tests are correct.Tests properly verify that this holiday is only observed from 2014 onwards, as specified in the implementation.
154-168
: Christmas Day tests are comprehensive.Tests properly verify both the regular holiday and observed dates when December 25 falls on a weekend.
169-180
: Boxing Day tests are comprehensive.Tests properly verify both the regular holiday and observed dates when December 26 falls on a weekend.
181-187
: Special holidays tests are included.Tests correctly verify the non-recurring special holidays like "Day after the General Election".
holidays/countries/antigua_and_barbuda.py (7)
1-21
: Import structure looks good.The file correctly imports the necessary modules and constants from the holidays package that will be needed for implementing Antigua and Barbuda's holiday rules.
23-33
: Well-documented class with proper references.The class includes thorough documentation with official references to the legal documents that define Antigua and Barbuda's holidays. This documentation approach is excellent for maintenance and verification.
35-38
: Class attributes look correct.The country code "AG" matches ISO standards, and the start year of 1955 appears appropriate considering the Public Holidays Act reference from 1954.
39-45
: Constructor implementation is correct.The initialization properly calls parent class initializers and sets the default observed rule to SUN_TO_NEXT_MON, which is appropriate for handling holidays that fall on Sundays.
46-96
: Holiday definitions look thorough and properly implemented.The implementation correctly applies different rules based on the year, matching the legislative changes mentioned in the references (2005 and 2014 amendments).
The code properly handles special cases like:
- Different observation rules for different holidays
- Year-dependent holiday names (e.g., "National Heroes Day" to "Sir Vere Cornwall Bird (SNR) Day")
- Additions and removals of holidays over time
The observation rules (SAT_SUN_TO_NEXT_MON, SAT_SUN_TO_NEXT_MON_TUE, etc.) are applied appropriately for each holiday.
99-105
: Class aliases are correctly implemented.The AG and ATG classes properly extend the main AntiguaAndBarbuda class to provide ISO 2-letter and 3-letter country code aliases.
107-123
: Static holidays are well documented and implemented.The AntiguaAndBarbudaStaticHolidays class correctly implements special holidays that occur on specific dates, particularly the day after the general election, with good documentation and references.
I particularly appreciate that you've included specific election years (2018, 2023) with exact dates rather than trying to calculate them programmatically, which would be complex and error-prone.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2395 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 207 208 +1
Lines 13185 13244 +59
Branches 1887 1899 +12
=========================================
+ Hits 13185 13244 +59 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
@kritibirda26, it's just great work! Just some technical suggestions and refactoring:
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Kriti Birda <164247895+kritibirda26@users.noreply.github.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
🔭 Outside diff range comments (1)
tests/countries/test_antigua_and_barbuda.py (1)
189-194
: 🧹 Nitpick (assertive)Additional (optional) data suggestions.
Currently, the day after a general election is tested for 2018 and 2023. If future elections occur, adding those dates would maintain the test coverage.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/antigua_and_barbuda.py
(1 hunks)tests/countries/test_antigua_and_barbuda.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/antigua_and_barbuda.py (1)
Learnt from: kritibirda26
PR: vacanza/holidays#2395
File: holidays/countries/antigua_and_barbuda.py:68-75
Timestamp: 2025-03-31T14:19:12.968Z
Learning: For Antigua and Barbuda holidays, Carnival Tuesday should only be considered a holiday for years after 2005 (not including 2005 itself) because the 2005 amendment was passed on September 15th, which is after the August Carnival celebrations that year.
🧬 Code Definitions (1)
tests/countries/test_antigua_and_barbuda.py (1)
holidays/countries/antigua_and_barbuda.py (3)
AntiguaAndBarbuda
(23-95)AG
(98-99)ATG
(102-103)
🪛 Ruff (0.8.2)
holidays/countries/antigua_and_barbuda.py
19-19: holidays.observed_holiday_base.MON_TO_NEXT_TUE
imported but unused
Remove unused import: holidays.observed_holiday_base.MON_TO_NEXT_TUE
(F401)
🔇 Additional comments (6)
tests/countries/test_antigua_and_barbuda.py (2)
13-29
: All tests are well-structured and comprehensive.The setup relies on
CommonCountryTests
to ensure consistency across test classes and usesunittest.TestCase
effectively. Testing the absence of holidays before 1955 is also an excellent approach.
30-188
: Test coverage is thorough for all major holidays.Each holiday is validated across a wide range of years, including checks for observed vs. non-observed dates. This approach provides strong confidence in correctness.
holidays/countries/antigua_and_barbuda.py (4)
1-22
: Documentation and references look correct.All law references are accurately documented. The class naming is clear, and the start year aligns with the official data. No issues found here.
🧰 Tools
🪛 Ruff (0.8.2)
19-19:
holidays.observed_holiday_base.MON_TO_NEXT_TUE
imported but unusedRemove unused import:
holidays.observed_holiday_base.MON_TO_NEXT_TUE
(F401)
23-72
: Holiday population logic adheres to legislative acts.
- The separation of holidays before and after 2005 is consistent with the recognized effective dates.
- The Carnival Tuesday logic appropriately starts from 2006, reflecting the late passage of the 2005 amendment.
73-104
: Observed holiday logic is properly accounted for.Observed rules (e.g.,
SAT_SUN_TO_NEXT_MON_TUE
) are correctly imported and applied for Christmas Day and Boxing Day.
106-124
: Static holiday definitions are accurate.The
AntiguaAndBarbudaStaticHolidays
class neatly centralizes special election holidays data. No concerns here.
I found a few more legislative acts on Antigua holidays:
|
Update the holidays according the new acts (skipped the half day). |
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)
holidays/countries/antigua_and_barbuda.py (1)
80-87
: 🧹 Nitpick (assertive)Independence Day implementation is functional but could be refactored
The implementation correctly handles the different observed rules based on year (>= 2005), but duplicates the holiday creation call.
Consider refactoring to store the date first and then apply observations conditionally:
- # Independence Day. - independence_day_name = "Independence Day" - if self._year >= 2005: - self._add_observed( - self._add_holiday_nov_1(independence_day_name), rule=SAT_SUN_TO_NEXT_MON - ) - else: - self._add_holiday_nov_1(independence_day_name) + # Independence Day. + dt = self._add_holiday_nov_1("Independence Day") + if self._year >= 2005: + self._add_observed(dt, rule=SAT_SUN_TO_NEXT_MON)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/antigua_and_barbuda.py
(1 hunks)tests/countries/test_antigua_and_barbuda.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/antigua_and_barbuda.py (1)
Learnt from: kritibirda26
PR: vacanza/holidays#2395
File: holidays/countries/antigua_and_barbuda.py:68-75
Timestamp: 2025-03-31T14:19:12.968Z
Learning: For Antigua and Barbuda holidays, Carnival Tuesday should only be considered a holiday for years after 2005 (not including 2005 itself) because the 2005 amendment was passed on September 15th, which is after the August Carnival celebrations that year.
🧬 Code Definitions (1)
tests/countries/test_antigua_and_barbuda.py (1)
holidays/countries/antigua_and_barbuda.py (3)
AntiguaAndBarbuda
(22-112)AG
(115-116)ATG
(119-120)
🔇 Additional comments (29)
tests/countries/test_antigua_and_barbuda.py (18)
1-16
: LGTM - Well-structured imports and test class setupThe imports are correctly set up with TestCase from unittest, the necessary holiday classes, and the CommonCountryTests. The test class inherits from both CommonCountryTests and TestCase, which follows the project's testing patterns.
17-25
: Good setup for testing periodsThe setup correctly initializes testing for years 1955 to 2050, covering the entire period from the country's start_year. The test_country_aliases method properly verifies that AG and ATG are aliases for AntiguaAndBarbuda.
26-28
: Good boundary testingTesting that no holidays exist before the start_year is an important edge case to verify.
29-41
: Special holidays are correctly testedYou've included all the one-time holidays referenced in the legislation, including:
- The 1993 public holiday
- The 2008 state funerals
- Days after general elections (2018, 2023)
This aligns well with the legislative references mentioned in the PR comments.
43-55
: Robust test for New Year's DayThe test correctly verifies:
- New Year's Day across all years in range 6D40 li>
- Observed date cases when Jan 1 falls on a Sunday
- That the non-observed holiday doesn't exist on observed dates
The test cases cover multiple years with different scenarios.
56-67
: Good Friday test looks goodThis test appropriately checks for Good Friday across multiple explicit years (2021-2025) as well as the entire range of years.
68-79
: Easter Monday test is well-implementedSimilar to Good Friday, this test checks specific dates for Easter Monday across several years, along with verifying it exists for the entire testing period.
80-91
: Labour Day test is comprehensiveTesting first Monday of May across multiple years and the full range.
92-103
: Whit Monday test looks goodAppropriate test coverage for this moveable feast day.
104-120
: Caribbean Community Day test is thoroughThe test correctly verifies:
- The holiday exists from 1955-2005
- The holiday doesn't exist from 2006 onwards
- Specific date assertions for several years in the valid range
This matches the 2005 amendment that removed this holiday.
121-132
: Carnival Monday test is completeTest coverage for all years is appropriate.
133-145
: Carnival Tuesday test correctly handles the legislative changeThe test properly verifies:
- The holiday exists from 2006 onwards
- The holiday doesn't exist from 1955-2005
This aligns with the 2005 amendment (enacted after the August carnival) that introduced this holiday.
146-159
: Independence Day test is thoroughThe test appropriately checks:
- The holiday exists for all years in the range
- Observed date cases when Nov 1 falls on a weekend (from 2005 onwards)
- That the non-observed holiday doesn't exist on observed dates
The test cases include years before and after the 2005 change to observation rules.
160-178
: National Heroes Day test is detailedThe test correctly verifies:
- The holiday exists for 2005-2013
- Special cases for 2006 (Dec 11) and 2012 (Dec 10)
- The holiday doesn't exist before 2005 or after 2013
This matches the legislative history of this holiday.
179-186
: Sir Vere Cornwall Bird SNR. Day test is well-implementedThe test correctly verifies:
- The holiday exists from 2014 onwards
- The holiday doesn't exist before 2014
- Observed date cases for when Dec 9 falls on a weekend (post-2019)
This aligns with the 2014 and 2019 amendments.
187-200
: Christmas Day test includes observance rulesThe test properly checks:
- The holiday exists for all years in the range
- Observed date cases when Dec 25 falls on a weekend
- That the non-observed holiday doesn't exist on observed dates
The test cases cover the SAT_SUN_TO_NEXT_MON_TUE observation rule.
201-211
: Boxing Day test is comprehensiveSimilar to Christmas Day, this test properly covers both the standard holiday and observed cases.
212-228
: Year-specific test for 2025 is valuableTesting all expected holidays for a specific future year is a good approach to verify the overall functionality. This serves as an integration test making sure all holidays interact properly in a single year.
holidays/countries/antigua_and_barbuda.py (11)
1-20
: Imports and license are well organizedThe file correctly includes imports for:
- Calendar constants
- Holiday groups
- Observed holiday base and rules
All necessary imports are present for the implementation.
22-40
: Class documentation is thoroughThe documentation includes:
- Clear description
- Comprehensive legislative references with links
- Notes about special cases (2006 and 2012 for National Heroes Day)
This level of documentation makes the code maintainable and verifiable.
42-51
: Class setup and initialization are properly implementedThe country code, observed label, and start year are correctly defined. The initialization method properly:
- Initializes the parent classes
- Sets up static holidays
- Establishes the default observed rule
- Calls the parent initializer
This follows the project's patterns for holiday classes.
53-67
: Standard annual holidays are correctly implementedThe implementation for fixed holidays (New Year's Day) and Christian holidays (Good Friday, Easter Monday, Whit Monday) is correct, along with the Labour Day on the first Monday of May.
74-78
: Carnival celebrations are correctly implementedCarnival Monday applies to all years, while Carnival Tuesday is correctly limited to 2006 onwards as per the retrieved learning that notes the 2005 amendment was passed after the August carnival that year.
89-97
: National Heroes Day logic is correctly implementedThe implementation properly handles:
- Special case for 2006 (Dec 11)
- Special case for 2012 (Dec 10)
- Standard case for 2005-2013 (Dec 9)
This matches the legislative history in the documentation.
98-107
: Sir Vere Cornwall Bird SNR. Day implementation is correctThe implementation properly:
- Limits this holiday to 2014 onwards
- Applies the observed rule only after 2019
- Uses the SAT_SUN_TO_NEXT_MON rule for observation
This aligns with the 2014 and 2019 amendments.
108-112
: Christmas and Boxing Day implementation follows project patternsBoth holidays use the SAT_SUN_TO_NEXT_MON_TUE observation rule which is correct for these holidays.
115-120
: Country code aliases are correctly implementedThe AG (alpha-2) and ATG (alpha-3) country code aliases are properly defined.
123-135
: Static holidays class has excellent documentationThe documentation includes:
- Clear description
- Legislative references with links for special holidays
- Explanation for the day after general election holiday
This thorough documentation makes the code maintainable and verifiable.
137-150
: Special holidays are well implementedThe implementation includes all the one-time holidays mentioned in the PR comments and legislation:
- August 3, 1993 public holiday
- 2008 state funerals (Feb 19 and Mar 18)
- Days after general elections (2018 and 2023)
This aligns with the legislative references and PR objectives.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Kriti Birda <164247895+kritibirda26@users.noreply.github.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)
tests/countries/test_antigua_and_barbuda.py
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/countries/test_antigua_and_barbuda.py (1)
holidays/countries/antigua_and_barbuda.py (3)
AntiguaAndBarbuda
(23-107)AG
(110-111)ATG
(114-115)
🔇 Additional comments (9)
tests/countries/test_antigua_and_barbuda.py (9)
13-13
: Import statement correctly updatedThe correct import statement is used for TestCase as suggested in previous reviews.
22-23
: LGTM: Setup looks goodThe setup correctly configures the test class for years between 1955 and 2050, matching the start_year defined in the Antigua and Barbuda implementation.
28-30
: LGTM: Good test for boundary conditionExcellent test to verify that no holidays exist before the start year (1954), which matches the implementation's start_year of 1955.
31-41
: LGTM: Special holidays correctly implementedThe test for special holidays properly verifies one-time holidays including state funerals and post-election days. Good use of a variable for the repeated "Day after the General Election" holiday name.
42-54
: LGTM: New Year's Day test looks correctThe test properly verifies that New Year's Day is observed on January 1st for all years in range, with correct observation rules when January 1st falls on a Sunday.
103-114
: LGTM: Caribbean Community Day test looks completeThe test correctly verifies that Caribbean Community Day was observed between 1955 and 2006, and not observed after 2006, which matches the implementation in the main code.
127-139
: LGTM: Carnival Tuesday test handles date range correctlyThe test properly verifies that Carnival Tuesday is observed from 2006 onwards and not observed before 2006, which matches the implementation in the main code.
154-177
: LGTM: National Heroes and Sir Vere Cornwall Bird Snr. Day test is thoroughThe test correctly handles the name change from National Heroes Day to Sir Vere Cornwall Bird Snr. Day in 2014, and includes the special cases for 2006 and 2012 as mentioned in the implementation documentation.
203-218
: LGTM: 2025 holiday test is completeThe test for 2025 holidays correctly includes all expected holidays for that year, including the observed Independence Day on November 3rd (since November 1st falls on a Saturday).
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. It's just exellent work, in terms of speed and quality! 👍
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.
Thanks for another great contribution @kritibirda26 !
Proposed change
Add holidays for Antigua and Barbuda
Resolves #1146.
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green