-
-
Notifications
You must be signed in to change notification settings - Fork 558
Update North Macedonia holidays, add l10n support #2554
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 update introduces comprehensive support for North Macedonia holidays. It expands and refactors the holiday logic, adds new ethnic and religious holiday categories, implements detailed localization in Macedonian, English, and Ukrainian, and revises the holiday dataset. The test suite is rewritten to cover all new features and localizations. Changes
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ 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
|
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2554 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 219 219
Lines 13956 14033 +77
Branches 1956 1975 +19
=========================================
+ Hits 13956 14033 +77 ☔ 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: 3
🔭 Outside diff range comments (1)
holidays/locale/mk/LC_MESSAGES/MK.po (1)
1-148
:⚠️ Potential issueMacedonian locale file missing translations.
Allmsgstr
fields are empty, so users of themk
locale will see no holiday names. Populate eachmsgstr
(typically identical to itsmsgid
) to provide proper Macedonian translations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (8)
README.md
(1 hunks)holidays/constants.py
(1 hunks)holidays/countries/north_macedonia.py
(3 hunks)holidays/locale/en_US/LC_MESSAGES/MK.po
(1 hunks)holidays/locale/mk/LC_MESSAGES/MK.po
(1 hunks)holidays/locale/uk/LC_MESSAGES/MK.po
(1 hunks)snapshots/countries/MK_COMMON.json
(1 hunks)tests/countries/test_north_macedonia.py
(1 hunks)
🔇 Additional comments (7)
holidays/constants.py (1)
60-67
: Global holiday category constants added.
The new constants (ALBANIAN
,ARMENIAN
,BOSNIAN
,ROMA
,SERBIAN
,TURKISH
,VLACH
) are introduced alphabetically and follow the existing lowercase naming convention. Please ensure these categories are consumed by the North Macedonia country class’ssupported_categories
list.README.md (1)
966-967
: README updated for North Macedonia.
The “Supported Languages” and “Supported Categories” columns now correctly listen_US, mk, uk
and the expanded holiday categories (ALBANIAN, BOSNIAN, CATHOLIC, HEBREW, ISLAMIC, ORTHODOX, ROMA, SERBIAN, TURKISH, VLACH
), matching the code changes in constants and locale files.holidays/locale/en_US/LC_MESSAGES/MK.po (1)
1-148
: New English localization file appears complete.
Allmsgstr
entries are populated appropriately, and header metadata (Project-Id-Version, language, creation date) is correct. No untranslated strings detected.holidays/locale/uk/LC_MESSAGES/MK.po (1)
1-148
: Ukrainian localization added successfully.
Themsgstr
entries are fully populated with accurate Ukrainian translations, including status annotations(приблизна дата)
and(вихідний)
. Header metadata is in order.snapshots/countries/MK_COMMON.json (1)
1-15
: Verify snapshot language consistency with new defaultmk
.The snapshot still stores English labels while
NorthMacedonia.default_language
is now"mk"
.
Double-check that the snapshot generator is explicitly instantiated withlanguage="en_US"
; otherwise, the CI “snapshot compare” step may start failing.
No action needed if the generator already forces the locale.holidays/countries/north_macedonia.py (1)
264-303
:❓ Verification inconclusive
Extend fixed Islamic date tables beyond 2025.
Static mappings stop at 2025, yet the module serves data to 2050.
After 2025 every Eid is flagged estimated, sacrificing accuracy.
If authoritative dates are available, pre-populate the tables:+ 2029: (APR, 24), + 2030: (APR, 13), + # … up to 2050Apply the same treatment to
EID_AL_FITR_DATES
.
Extend static Islamic holiday mappings through 2050
The
NorthMacedoniaIslamicHolidays
class currently hard-codes Eid al-Adha and Eid al-Fitr only up to 2025; after that the library falls back to estimated dates. If you have authoritative dates through 2050, please add them to improve accuracy.• File:
holidays/countries/north_macedonia.py
Lines: 264–303Suggested diff format:
class NorthMacedoniaIslamicHolidays(_CustomIslamicHolidays): EID_AL_ADHA_DATES = { …, 2025: (JUN, 6), + 2026: (MAY, 27), + 2027: (MAY, 17), + 2028: (MAY, 6), + 2029: (APR, 24), + # … continue through 2050 } EID_AL_FITR_DATES = { …, 2025: (MAR, 30), + 2026: (MAR, 20), + 2027: (MAR, 10), + 2028: (FEB, 27), + 2029: (FEB, 16), + # … continue through 2050 }Please confirm you have reliable sources for the post-2025 dates before merging.
tests/countries/test_north_macedonia.py (1)
110-112
:range()
objects may not be whatassertHolidayName
expects.Several assertions pass
range(...)
directly (e.g.range(2007, 2050)
).
Unless the helper expands those integers into concrete ISO-date strings, the comparison is effectively no-op.
Please verify and, if needed, replace with an explicit generator:(f"{year}-04-20" for year in range(2007, 2050))
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, great work as always 🇲🇰
Proposed change
Update North Macedonia holidays, add l10n support (en_US, mk, uk).
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green