-
-
Notifications
You must be signed in to change notification settings - Fork 550
Add Saint Vincent and the Grenadines holidays #2608
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
Summary by CodeRabbit
WalkthroughSupport for Saint Vincent and the Grenadines holidays has been added. This includes a new holiday provider class, registry updates, country imports, localization files for English, documentation updates, and a dedicated test suite to verify holiday calculations and aliases. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2608 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 256 257 +1
Lines 15358 15399 +41
Branches 2104 2105 +1
=========================================
+ Hits 15358 15399 +41 ☔ 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.
Actionable comments posted: 4
📜 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/saint_vincent_and_the_grenadines.py
(1 hunks)holidays/locale/en_US/LC_MESSAGES/VC.po
(1 hunks)holidays/locale/en_VC/LC_MESSAGES/VC.po
(1 hunks)holidays/registry.py
(1 hunks)tests/countries/test_saint_vincent_and_the_grenadines.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
holidays/countries/__init__.py (1)
holidays/countries/saint_vincent_and_the_grenadines.py (3)
SaintVincentAndTheGrenadines
(22-91)VC
(94-95)VCT
(98-99)
tests/countries/test_saint_vincent_and_the_grenadines.py (2)
tests/common.py (6)
CommonCountryTests
(356-374)assertAliases
(121-130)assertNoHolidays
(292-294)assertHolidayName
(195-199)assertNoNonObservedHoliday
(248-250)assertHolidays
(228-230)holidays/countries/saint_vincent_and_the_grenadines.py (3)
SaintVincentAndTheGrenadines
(22-91)VC
(94-95)VCT
(98-99)
🪛 Pylint (3.3.7)
tests/countries/test_saint_vincent_and_the_grenadines.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 23-23: Missing class docstring
(C0115)
[warning] 25-25: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSaintVincentAndTheGrenadines.setUpClass' method
(W0221)
[convention] 29-29: Missing function or method docstring
(C0116)
[convention] 32-32: Missing function or method docstring
(C0116)
[convention] 35-35: Missing function or method docstring
(C0116)
[convention] 47-47: Missing function or method docstring
(C0116)
[convention] 59-59: Missing function or method docstring
(C0116)
[convention] 69-69: Missing function or method docstring
(C0116)
holidays/countries/saint_vincent_and_the_grenadines.py
[convention] 30-30: Line too long (112/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 94-94: Missing class docstring
(C0115)
[convention] 98-98: Missing class docstring
(C0115)
🔇 Additional comments (16)
README.md (1)
108-108
: Update supported country count
The count has been updated to 184, matching the new addition. Ensure this manual update stays in sync when adding/removing countries.holidays/countries/__init__.py (1)
156-156
: Import the new country provider
The import forSaintVincentAndTheGrenadines
looks correct and is placed in alphabetical order.holidays/locale/en_VC/LC_MESSAGES/VC.po (1)
1-81
: Template for en_VC locale added
The new.po
file correctly defines all message IDs for the en_VC variant. Ensure a final newline is present at end of file.holidays/locale/en_US/LC_MESSAGES/VC.po (1)
46-48
: Verify translation for National Workers Day
Using "Labor Day" may conflict with the US holiday on the first Monday of September. Confirm that this translation reflects the May 1 observance rather than the US Labor Day.holidays/countries/saint_vincent_and_the_grenadines.py (5)
22-37
: Well-structured holiday provider class with solid foundation.The class definition follows the established patterns with appropriate inheritance and configuration. The start year of 1979 aligns with Saint Vincent and The Grenadines' independence.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 30-30: Line too long (112/100)
(C0301)
39-43
: Constructor properly initializes parent classes.The initialization sequence and default observed rule setup look good.
62-63
: Good conditional logic for future holiday.The year check for National Spiritual Baptist Day starting in 2025 is well-implemented.
68-79
: Excellent handling of COVID-19 adjustments for 2020.The special case for Carnival Monday/Tuesday in 2020 with proper documentation link shows good attention to real-world changes.
94-99
: Standard alias implementation.The VC and VCT alias classes follow the established pattern for country code aliases.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 94-94: Missing class docstring
(C0115)
[convention] 98-98: Missing class docstring
(C0115)
tests/countries/test_saint_vincent_and_the_grenadines.py (7)
23-27
: Proper test setup with comprehensive year range.The test class setup correctly inherits from CommonCountryTests and covers the full supported year range from 1979 to 2049.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 23-23: Missing class docstring
(C0115)
[warning] 25-25: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSaintVincentAndTheGrenadines.setUpClass' method
(W0221)
29-30
: Alias testing looks solid.The country alias test properly verifies both VC and VCT aliases.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Missing function or method docstring
(C0116)
32-33
: Good boundary testing for start year.Testing that no holidays exist before 1979 ensures the start_year logic works correctly.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 32-32: Missing function or method docstring
(C0116)
35-45
: Thorough observed holiday testing for New Year's Day.The observed date calculations are accurate - all test dates correctly represent years where January 1st fell on Sunday, requiring Monday observation.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 35-35: Missing function or method docstring
(C0116)
47-57
: Comprehensive Labor Day testing.The observed date logic for National Workers Day is correct - all dates represent May 1st falling on Sunday with proper Monday observation.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 47-47: Missing function or method docstring
(C0116)
59-67
: Accurate Emancipation Day testing.The observed dates for August 1st falling on Sunday are correctly calculated.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 59-59: Missing function or method docstring
(C0116)
69-84
: Excellent comprehensive test for 2024.This test covers all holidays defined in the provider and serves as a good regression test. The dates and holiday names match the expected implementation.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 69-69: Missing function or method docstring
(C0116)
Fixed Typo Issues Signed-off-by: Muhammad Waqar <[email protected]>
Fixed Typo Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Muhammad Waqar <[email protected]>
Signed-off-by: Muhammad Waqar <[email protected]>
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 (4)
README.md
(2 hunks)holidays/countries/saint_vincent_and_the_grenadines.py
(1 hunks)holidays/locale/en_US/LC_MESSAGES/VC.po
(1 hunks)holidays/registry.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
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.
🪛 Pylint (3.3.7)
holidays/countries/saint_vincent_and_the_grenadines.py
[convention] 30-30: Line too long (112/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 94-94: Missing class docstring
(C0115)
[convention] 98-98: Missing class docstring
(C0115)
🔇 Additional comments (4)
README.md (1)
108-108
: LGTM! Documentation properly updated for new country support.The country count increment and table entry are accurate and well-formatted.
Also applies to: 1137-1142
holidays/registry.py (1)
164-164
: Registry entry looks good.The entry follows the established pattern and the class name capitalization issue from previous reviews has been properly resolved.
holidays/locale/en_US/LC_MESSAGES/VC.po (1)
47-48
: Good localization choice.Translating "National Workers Day" to "Labor Day" for the en_US locale is culturally appropriate and follows American English conventions.
holidays/countries/saint_vincent_and_the_grenadines.py (1)
22-92
: Solid holiday provider implementation.The holiday definitions are comprehensive and well-structured. Good handling of special cases like the 2020 Carnival dates and the phased introduction of National Spiritual Baptist Day from 2025.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 30-30: Line too long (112/100)
(C0301)
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 (3)
README.md
(1 hunks)holidays/countries/__init__.py
(1 hunks)holidays/registry.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
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.
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (1)
holidays/countries/zimbabwe.py (3)
Zimbabwe
(17-79)ZW
(82-83)ZWE
(86-87)
🪛 Ruff (0.11.9)
holidays/countries/__init__.py
213-213: No newline at end of file
Add trailing newline
(W292)
🪛 Pylint (3.3.7)
holidays/countries/__init__.py
[convention] 213-213: Final newline missing
(C0304)
🔇 Additional comments (2)
holidays/registry.py (1)
165-165
: Registry entry looks solidThe new entry follows the established pattern perfectly - proper alphabetical placement, correct class name capitalization, and appropriate country codes.
README.md (1)
1144-1149
: Documentation entry is well-formattedThe new table row maintains proper alphabetical ordering and follows the established format. Good to see the supported languages are properly spaced.
Signed-off-by: Muhammad Waqar <[email protected]>
Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: Panpakorn Siripanich <[email protected]> Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: Panpakorn Siripanich <[email protected]> Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: Panpakorn Siripanich <[email protected]> Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: ~Jhellico <[email protected]> Signed-off-by: Muhammad Waqar <[email protected]>
Co-authored-by: ~Jhellico <[email protected]> Signed-off-by: Muhammad Waqar <[email protected]>
Signed-off-by: Muhammad Waqar <[email protected]>
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.
This solution can be used in other places! 🚀
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
Thanks for adding this @waqar2403 !
Proposed change
Added Holidays for Saint Vincent and The Grenadines
Closes: #1263
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green