-
-
Notifications
You must be signed in to change notification settings - Fork 543
Add Qatar holidays #2409
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
Add Qatar holidays #2409
Conversation
Summary by CodeRabbit
WalkthroughThis update extends holiday management support to Qatar. The README now reflects Qatar’s inclusion with an updated country code count and detailed entry. A new module for Qatar is introduced, along with corresponding export adjustments in the package’s initializer and registry. Localization files for Arabic and English are added, and a comprehensive test suite for Qatar’s holiday logic has been implemented. Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)holidays/countries/qatar.py (1)
🔇 Additional comments (5)
✨ 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: 4
🔭 Outside diff range comments (1)
holidays/locale/en_US/LC_MESSAGES/QA.po (1)
1-62
:⚠️ Potential issueFuture dates in PO file metadata need correction.
The localization file is well-structured with proper translations for all Qatar holidays. However, the creation and revision dates are set to April 2025, which is in the future.
Apply this change to correct the dates:
-"POT-Creation-Date: 2025-04-03 09:48+0500\n" -"PO-Revision-Date: 2025-04-03 10:42+0500\n" +"POT-Creation-Date: 2024-04-03 09:48+0500\n" +"PO-Revision-Date: 2024-04-03 10:42+0500\n"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
README.md
(2 hunks)holidays/countries/__init__.py
(1 hunks)holidays/countries/qatar.py
(1 hunks)holidays/locale/ar_QA/LC_MESSAGES/QA.po
(1 hunks)holidays/locale/en_US/LC_MESSAGES/QA.po
(1 hunks)holidays/registry.py
(1 hunks)tests/countries/test_qatar.py
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
holidays/countries/__init__.py (1)
holidays/countries/qatar.py (3)
Qatar
(48-104)QA
(107-108)QAT
(111-112)
tests/countries/test_qatar.py (1)
holidays/countries/qatar.py (3)
Qatar
(48-104)QA
(107-108)QAT
(111-112)
🔇 Additional comments (22)
holidays/registry.py (1)
146-146
: Good addition of Qatar to the COUNTRIES dictionary.The Qatar entry follows the correct format and is properly positioned alphabetically within the dictionary. This seamless integration will allow users to access Qatar holidays using the standard country codes.
holidays/countries/__init__.py (1)
138-138
: Import statement for Qatar properly integrated.The import for the Qatar module and its associated classes is correctly placed in alphabetical order, consistent with the project's import structure pattern.
holidays/locale/en_US/LC_MESSAGES/QA.po (1)
30-62
: Well-structured translations for Qatar holidays.The translations are clear and comprehensive, covering all holidays implemented in the Qatar class. The formatting follows the standard PO file pattern, making it easy to maintain and extend.
README.md (2)
102-102
: Correctly updated country code count.The country count has been properly updated from 163 to 164 to reflect the addition of Qatar.
1005-1010
: Qatar entry properly formatted in README table.The Qatar entry includes all necessary information (country name, code, supported languages, categories) and follows the same formatting pattern as other countries. The default language is correctly set to Arabic (ar_QA) with English (en_US) as an additional supported language.
holidays/locale/ar_QA/LC_MESSAGES/QA.po (3)
15-28
: PO file structure appears correct, but confirm empty msgstr fieldsThe PO file structure and metadata look good with proper language settings (ar_QA). However, all the translation strings (msgstr) are empty, which is unusual for a complete localization file.
In gettext localization, typically the msgid contains the source text and msgstr contains the translation. Here, Arabic text is already in msgid fields, suggesting a non-standard approach.
Are empty msgstr fields intentional for this project? Please check other localization files to confirm this pattern is consistent with the project's localization approach.
29-32
: Format string interpolation for estimatesThis correctly defines a format string for estimated holidays. The localization string includes the Arabic phrase for "estimated" that will be used when displaying estimated Islamic holiday dates.
34-60
: Holiday translations look completeAll necessary Qatar holidays have been included with proper Arabic names:
- National Sports Day
- Qatar National Day
- Eid al-Fitr
- Eid al-Adha
- New Year's Day
- New Year's Holiday
- March Bank Holiday
tests/countries/test_qatar.py (7)
47-73
: Sports Day test is thorough and accurateThe test thoroughly verifies National Sports Day dates from 2012 to 2030, correctly falling on the second Tuesday of February each year. It also checks that the holiday doesn't exist before 2012.
74-78
: National Day test is properly implementedThe test correctly verifies that Qatar National Day is celebrated on December 18th starting from 2007.
79-107
: Eid al-Fitr test uses accurate datesThe test properly verifies Eid al-Fitr dates for multiple years (2018-2025), including the three-day duration of the holiday.
109-135
: Eid al-Adha test uses accurate datesThe test properly verifies Eid al-Adha dates for multiple years (2018-2024), including the three-day duration of the holiday.
136-143
: Bank holiday tests properly configuredThe New Year's Day test correctly verifies the holiday exists for all years in bank holidays category only.
144-167
: March Bank Holiday test is comprehensiveTest thoroughly verifies that March Bank Holiday falls on the first Sunday of March from 2010 onward. It correctly checks it's only in the bank category and doesn't exist in public holidays.
186-214
: Localization tests are thoroughThe tests properly verify both default Arabic (ar_QA) and English (en_US) translations of all holidays, ensuring the localization works correctly.
holidays/countries/qatar.py (7)
48-63
: Class attributes are well-definedThe Qatar class is well-structured with appropriate attributes:
- Correct country code "QA"
- Default language set to Arabic (ar_QA)
- Support for both Arabic and English
- Weekend correctly defined as Friday and Saturday
- Appropriate categories for both public and bank holidays
- Start year set to 1971 (Qatar's independence)
64-76
: Constructor properly initializes parent classesThe constructor properly initializes Islamic and Static holidays with appropriate parameters. The islamic_show_estimated parameter allows flexibility in displaying estimated dates.
77-97
: Public holidays implementation looks correctThe implementation of public holidays follows best practices:
- National Sports Day starts from 2012 (second Tuesday of February)
- Qatar National Day starts from 2007 (December 18)
- Islamic holidays (Eid al-Fitr and Eid al-Adha) are properly implemented with 3-day duration
98-105
: Bank holidays correctly implementedBank holidays are properly implemented:
- New Year's Day (January 1)
- March Bank Holiday (first Sunday of March) starting from 2010
107-113
: Country code aliases defined correctlyCountry code aliases QA and QAT are defined correctly to allow for flexible usage of the Qatar holiday class.
115-164
: Islamic holiday dates are comprehensiveThe QatarIslamicHolidays class provides specific dates for Eid al-Fitr and Eid al-Adha from 2005 to 2025, which helps ensure accurate holiday calculations. The reference to timeanddate.com provides appropriate attribution.
166-177
: Static holidays implementation with documentationThe QatarStaticHolidays class properly defines a special New Year's Holiday for 2025 with appropriate reference documentation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2409 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 209 210 +1
Lines 13301 13348 +47
Branches 1906 1909 +3
=========================================
+ Hits 13301 13348 +47 ☔ 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.
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 👍
Thanks for taking care of this @Wasif-Shahzad
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.
Sorry about the late change requests - but I just remembered about Qatar's weekend
change when we were combing through them a while back.
Other than that, the implementation in this PR looks solid 👍
Head branch was pushed to by a user without write access
|
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 🇶🇦
Add Qatar Holidays
I've added Qatar holidays. It included Bank Holidays too. I've added
l10n
support in ar_QA and en_US as well.Closes #1253.
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green