Add Iraq holidays #2438
Add Iraq holidays #2438
Conversation
Summary by CodeRabbit
WalkthroughThis pull request introduces support for Iraq within the project by adding a new contributor, updating the README to reflect an increase in supported country codes, and implementing a comprehensive holiday management system for Iraq. This includes the addition of new classes, import statements, registry entries, and localization files for both Arabic and English. Additionally, a suite of tests has been created to validate the functionality related to Iraq's holidays. Changes
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Definitions (1)holidays/countries/__init__.py (1)
🔇 Additional comments (3)
✨ 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.
Copilot reviewed 6 out of 8 changed files in this pull request and generated no comments.
Files not reviewed (2)
- holidays/locale/ar_IQ/LC_MESSAGES/IQ.po: Language not supported
- holidays/locale/en_US/LC_MESSAGES/IQ.po: Language not supported
Comments suppressed due to low confidence (1)
holidays/countries/iraq.py:62
- The class name 'IraqIslamicCalender' appears to be misspelled. Consider renaming it to 'IraqIslamicCalendar' for consistency.
IslamicHolidays.__init__(self, cls=IraqIslamicCalender, show_estimated=islamic_show_estimated)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
AUTHORS.md(1 hunks)README.md(2 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/iraq.py(1 hunks)holidays/locale/ar_IQ/LC_MESSAGES/IQ.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/IQ.po(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_iraq.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
holidays/countries/__init__.py (1)
holidays/countries/iraq.py (3)
Iraq(45-151)IQ(154-155)IRQ(158-159)
🔇 Additional comments (17)
AUTHORS.md (1)
22-22: Well-placed author additionThe author's name "Ashaz Akhtar" has been correctly added in alphabetical order within the list of contributors.
holidays/countries/__init__.py (1)
88-88: Clean import additionThe import for Iraq and its country codes follows the established pattern and maintains alphabetical ordering between Iran and Ireland.
holidays/registry.py (1)
97-97: Proper registry entryThe Iraq entry in the COUNTRIES dictionary maintains consistent formatting with the rest of the registry and is correctly positioned alphabetically.
README.md (2)
102-102: Accurate country count updateThe country count has been properly incremented from 164 to 165 to reflect the addition of Iraq.
656-661: Correctly formatted country entryThe Iraq entry follows the established table format and contains all required information including supported languages with proper default designation.
holidays/locale/en_US/LC_MESSAGES/IQ.po (2)
1-16: Metadata looks correct
No issues spotted. The header and license metadata align nicely with .po file conventions.
29-79: Translations appear consistent
All holiday names and translations look coherent, with no placeholder mismatches.tests/countries/test_iraq.py (3)
27-44: Tests thoroughly verify early years
Confirmed that holiday checks before 1933 are correctly handled, including boundary tests for 1932 and 1933.
50-70: Comprehensive holiday assertions
The range of tested holiday dates (e.g., Army Day, Islamic New Year, multiple-day Eids) shows strong coverage.
110-132: Coverage of special holidays
Easter, Sukkot, and Passover testing underinclude_specialensures clarity around optional holidays.holidays/countries/iraq.py (3)
45-59: Robust multi-inheritance approach
Extending multiple holiday groups aligns well with the library’s design. Theweekendset correctly reflects local practices.
87-108: Temporal disabling of estimated labels
Toggling_islamic_calendar_show_estimatedfor 2021 and 2022, then restoring its original value, is a neat solution.
137-145: Good handling of special holidays
Offering these only wheninclude_specialis set toTruekeeps the library flexible for various use cases.holidays/locale/ar_IQ/LC_MESSAGES/IQ.po (4)
30-32: Format string for estimated holidays.This format string accommodates holiday dates that may vary each year. No issues with implementation.
34-80: Holiday translations structure looks good.All Arabic holiday names are correctly set as message IDs. The empty msgstr entries are appropriate when msgid is already in the target language.
18-20:⚠️ Potential issueFuture dates detected in translation metadata.
The PO file headers contain dates from 2025, which is in the future. These should be updated to the current date.
-"Project-Id-Version: Holidays 0.70\n" -"POT-Creation-Date: 2025-04-05 12:08+0530\n" -"PO-Revision-Date: 2025-04-05 12:08+0530\n" +"Project-Id-Version: Holidays 0.70\n" +"POT-Creation-Date: 2023-08-01 12:08+0530\n" +"PO-Revision-Date: 2023-08-01 12:08+0530\n"⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2385 File: holidays/locale/en_US/LC_MESSAGES/PK.po:17-26 Timestamp: 2025-04-02T17:22:27.207Z Learning: Always verify the current year before flagging dates in project metadata as "future dates." As of March 2025, dates with year 2025 are current, not future dates.
1-81:❓ Verification inconclusive
Verify translation completeness.
The file contains key Iraqi holidays. Consider reviewing with a native speaker to confirm all major national and religious holidays are included.
🏁 Script executed:
#!/bin/bash # Check for similar files in other Arabic locales for comparison echo "Looking for other Arabic locale files for reference:" find holidays/locale -name "*.po" | grep -i "ar_" | grep -v "ar_IQ"Length of output: 180
Action: Manual Verification Required – Confirm Holiday Coverage
The IQ Arabic locale file includes key Iraqi holidays, but our search didn't reveal any other Arabic locale files for additional context. Please verify with a native speaker (or a trusted reference) that the list covers all major national and religious holidays for Iraq.
| ): | ||
| country = "IQ" | ||
| default_language = "ar_IQ" | ||
| estimated_label = tr("%s (تقديري)") |
There was a problem hiding this comment.
All localized strings shoul have localization comment in preceding line.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
holidays/locale/en_US/LC_MESSAGES/IQ.po (1)
35-81: 🛠️ Refactor suggestionProvide English translations for all Iraqi holidays
Eachmsgstris empty. Fill them in with correct English equivalents to ensure users see proper translations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/iraq.py(1 hunks)holidays/locale/ar_IQ/LC_MESSAGES/IQ.po(1 hunks)holidays/locale/en_US/LC_MESSAGES/IQ.po(1 hunks)
🔇 Additional comments (1)
holidays/countries/iraq.py (1)
60-60: Add translator comment for the estimated label
This aligns with the feedback from earlier reviews requiring a preceding localization comment for all translated strings.
| # Iraqi holidays translation. | ||
| # Copyright (C) 2024 Vacanza Team | ||
| # This file is distributed under the same license as the holidays package. | ||
| # |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove "fuzzy" marker if translations are final
Leaving "fuzzy" suggests incomplete or uncertain translations. Remove the marker if you are confident about the correctness.
| def _add_holiday_july_14(self, name): | ||
| """National Day""" | ||
| return self._add_holiday(name, JUL, 14) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clarify docstring usage for _add_holiday_july_14
Currently labeled "National Day," but also used for "Republic Day" in 2022. Update the docstring to reflect both names.
| def __init__(self, islamic_show_estimated: bool = True, *args, **kwargs): | ||
| """ | ||
| Args: | ||
| islamic_show_estimated: | ||
| Whether to add "estimated" label to Islamic holidays name | ||
| if holiday date is estimated. | ||
| """ |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Document 'include_special' parameter
While the docstring explains islamic_show_estimated, it omits details for _include_special. Consider adding a brief explanation for clarity.
| # License: MIT (see LICENSE file) | ||
| # SOME DESCRIPTIVE TITLE | ||
| # This file is distributed under the same license as the Holidays package. | ||
| # FIRST AUTHOR <ashazsyed105@gmail.com>, 2025. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Update author attribution
Replace the placeholder with your actual name or remove if not necessary.
| "Project-Id-Version: Holidays 0.70\n" | ||
| "POT-Creation-Date: 2025-04-05 12:08+0530\n" | ||
| "PO-Revision-Date: 2025-04-05 12:08+0530\n" | ||
| "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Fill in translator name
"FULL NAME EMAIL@ADDRESS" should reflect the actual translator’s information.
|
|
@MRAshaz tomorrow is the deadline for this task (3 weeks) so I'll have to free this issue up to anyone willing to work on it. |
|
I'm closing this due to a missed deadline. Feel free to reopen if you have a ready for review code. |
Proposed change
Resolves #1203.
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green