Update Mauritania holidays: add l10n support#3660
Conversation
WalkthroughAdds gettext-based localization for Mauritania holidays, introduces Arabic, en_MR, and en_US locale catalogs, updates Mauritania tests for localized names, and corrects the README country table row. ChangesMauritania i18n support
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
holidays/countries/mauritania.py (1)
29-72: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAuthor the Mauritania labels in Arabic
default_language = "ar"means these names need Arabic source literals sotr()matches the default-language catalog. The current English holiday names and plainestimated_labelwill fall back to English for Arabic users; use Arabic literals here and wrapestimated_labelwithtr(...)as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/countries/mauritania.py` around lines 29 - 72, The Mauritania holiday labels are still defined with English source strings even though Mauritania’s default_language is set to Arabic, so they won’t resolve correctly through tr(). Update Mauritania’s holiday names in _populate_public_holidays() to use Arabic literals for each holiday label, and change estimated_label to be translated as well by wrapping it with tr(...). Use the existing Mauritania class symbols default_language, estimated_label, and _populate_public_holidays to locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@holidays/countries/mauritania.py`:
- Around line 29-72: The Mauritania holiday labels are still defined with
English source strings even though Mauritania’s default_language is set to
Arabic, so they won’t resolve correctly through tr(). Update Mauritania’s
holiday names in _populate_public_holidays() to use Arabic literals for each
holiday label, and change estimated_label to be translated as well by wrapping
it with tr(...). Use the existing Mauritania class symbols default_language,
estimated_label, and _populate_public_holidays to locate the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4305f79f-eb90-4460-a192-0fab8522393a
📒 Files selected for processing (3)
README.mdholidays/countries/mauritania.pyholidays/locale/ar/LC_MESSAGES/MR.po
|
@TaniaRocks, I recommend looking at some examples of adding l10n support: #3091, #3117, #3389, #3605. |
There was a problem hiding this comment.
IMO you'll need to include the Arabic source(s) you used for the holiday names as well (any news websites are fine, but government ones are preferred)
I would recommend running make l10n after make setup since that auto-populates the .po files for your l10n work here instead of copying them manually from elsewhere - see https://holidays.readthedocs.io/en/latest/contributing/#running-tests for more details.
For the list of supported languages, I think we can go with these 3 for now (or more if you happen to find more/be a native speaker for certain languages)
ar: Mauritania's national languageen_MR: previous holiday names wording, local to Mauritaniaen_US: library-wide fallback standard, for naming seeegypt.pyfor example - note that holiday names comment above needs to be standardised toen_USones as well.
Aside from that, test_l10n_default (for ar), test_l10n_en_mr, and test_l10n_en_us needs to be added to at the very end of the test_mauritania.py as well, see test_singapore.py implementation for example - let's use year 2024 for this and remove the old test_2024 test case
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
holidays/locale/ar/LC_MESSAGES/MR.po (1)
46-48: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMissing translator comment for Independence Day.
en_US/LC_MESSAGES/MR.pohas#. Independence Day.before this msgid, but it's missing here. The Python source (mauritania.py) has the# Independence Day.comment, so this catalog is out of sync — likelymake l10nwasn't rerun after the latest source edit.Based on learnings, translator comments in
.pofiles are auto-generated from source comments and should be fixed by regenerating viamake l10n, not hand-edited.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/locale/ar/LC_MESSAGES/MR.po` around lines 46 - 48, The MR.po catalog is missing the auto-generated translator comment for the Independence Day msgid, so it is out of sync with the source comments in mauritania.py. Regenerate the localization files with make l10n rather than editing MR.po by hand, and ensure the resulting entry for the "عيد الاستقلال" msgid includes the translated comment metadata that matches the en_US/LC_MESSAGES/MR.po catalog.Source: Learnings
holidays/locale/en_US/LC_MESSAGES/MR.po (1)
30-66: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winAll
msgstrentries are empty — English translations aren't actually provided.Every msgid in this catalog has
msgstr "". Sinceen_USisn't the default/source language, untranslated entries here mean gettext falls back to the Arabicmsgidat runtime, not English. This breaks the entire purpose of theen_USlocale and will maketest_l10n_en_usintests/countries/test_mauritania.pyfail (it expects "New Year's Day", "Labor Day", etc.).🌐 Example fix (populate actual English translations)
#. New Year's Day. msgid "رأس السنة الميلادية" -msgstr "" +msgstr "New Year's Day" #. Labor Day. msgid "عيد العمال" -msgstr "" +msgstr "Labor Day" #. Africa Day. msgid "يوم أفريقيا" -msgstr "" +msgstr "Africa Day" #. Independence Day. msgid "عيد الاستقلال" -msgstr "" +msgstr "Independence Day" #. Eid al-Fitr. msgid "عيد الفطر" -msgstr "" +msgstr "Eid al-Fitr" #. Eid al-Adha. msgid "عيد الأضحى" -msgstr "" +msgstr "Eid al-Adha" #. Muharram/Islamic New Year. msgid "رأس السنة الهجرية" -msgstr "" +msgstr "Islamic New Year" #. Prophet Muhammad's Birthday. msgid "المولد النبوي الشريف" -msgstr "" +msgstr "The Prophet's Birthday" #. %s (estimated). #, c-format msgid "%s (تقديري)" -msgstr "" +msgstr "%s (estimated)"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/locale/en_US/LC_MESSAGES/MR.po` around lines 30 - 66, Populate the empty msgstr values in the en_US translation catalog so the locale returns English strings instead of falling back to the Arabic msgid. Update each holiday entry in this file (for example the records for New Year's Day, Labor Day, Africa Day, Independence Day, Eid al-Fitr, Eid al-Adha, Muharram/Islamic New Year, and Prophet Muhammad's Birthday) by filling in the English translation in msgstr, keeping the existing msgid keys and c-format marker for the estimated date entry. This will align the MR.po catalog with the expectations in test_l10n_en_us and ensure the locale works as a true English translation set.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 1138-1140: The README table has an incorrect country label for the
MR row: update the entry in the localization/country mapping table so MR is
labeled Mauritania instead of Mauritius. Locate the MR row in README.md and
change only that row’s name to match the correct country while keeping the rest
of the table structure and locale values unchanged.
---
Outside diff comments:
In `@holidays/locale/ar/LC_MESSAGES/MR.po`:
- Around line 46-48: The MR.po catalog is missing the auto-generated translator
comment for the Independence Day msgid, so it is out of sync with the source
comments in mauritania.py. Regenerate the localization files with make l10n
rather than editing MR.po by hand, and ensure the resulting entry for the "عيد
الاستقلال" msgid includes the translated comment metadata that matches the
en_US/LC_MESSAGES/MR.po catalog.
In `@holidays/locale/en_US/LC_MESSAGES/MR.po`:
- Around line 30-66: Populate the empty msgstr values in the en_US translation
catalog so the locale returns English strings instead of falling back to the
Arabic msgid. Update each holiday entry in this file (for example the records
for New Year's Day, Labor Day, Africa Day, Independence Day, Eid al-Fitr, Eid
al-Adha, Muharram/Islamic New Year, and Prophet Muhammad's Birthday) by filling
in the English translation in msgstr, keeping the existing msgid keys and
c-format marker for the estimated date entry. This will align the MR.po catalog
with the expectations in test_l10n_en_us and ensure the locale works as a true
English translation set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 55ea916d-d0d9-415f-b42d-ed5ef605ecc6
📒 Files selected for processing (6)
CONTRIBUTORSREADME.mdholidays/countries/mauritania.pyholidays/locale/ar/LC_MESSAGES/MR.poholidays/locale/en_US/LC_MESSAGES/MR.potests/countries/test_mauritania.py
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
I doubt that English is used to any significant extent in Mauritania. The current English names seem to be just common terms. |
|
Thanks for the clarification. I added the localization following the existing naming pattern in the project. If different English names would be more appropriate for Mauritania, I'd be happy to revise them. |
I mean, English isn't widely used in Mauritania, so en_MR locale is redundant. |
KJhellico
changed the title
eeshsaxena
left a comment
eeshsaxena
left a comment
There was a problem hiding this comment.
Thanks for adding l10n for Mauritania! The mechanics look solid - default_language = "ar" matches Mauritania's official language, both non-default locales (en_MR, en_US) are fully translated, the #. context comments come through for every entry, and the test split into test_l10n_default / test_l10n_en_us / test_l10n_en_mr follows the usual pattern.
One consistency thing I noticed while comparing the two English .po files. en_MR and en_US agree on every holiday except the two Islamic-calendar ones:
| msgid | en_US |
en_MR |
|---|---|---|
| رأس السنة الهجرية | Islamic New Year |
Muharram/Islamic New Year |
| المولد النبوي الشريف | The Prophet's Birthday |
Prophet Muhammad's Birthday |
Since both are English locales and there's no real US-vs-Mauritania English naming difference for these, the split looks unintentional. It might also be worth aligning them with the names already used elsewhere in the library, which would keep these holidays consistent for anyone comparing countries in English:
Islamic New Year- used in 23en_USlocale files (vs.Muharram/Islamic New Year, which is new here).Prophet's Birthday- used in 39en_USlocale files; this PR usesThe Prophet's Birthday(en_US) andProphet Muhammad's Birthday(en_MR), neither of which matches the common form.
So aligning both locales to Islamic New Year and Prophet's Birthday would match the established convention. Not a blocker, and please disregard if the regional difference is deliberate - just flagging it since the rest of the file lines up so cleanly. Deferring to the maintainers on the naming call.
eeshsaxena
left a comment
eeshsaxena
left a comment
There was a problem hiding this comment.
Thanks for working on this. The Arabic default plus the en_US/en_MR split reads well and the test restructuring into test_l10n_default/test_l10n_en_us/test_l10n_en_mr follows the usual pattern. A couple of things I noticed while reading through:
1. en_US and en_MR disagree on two holiday names. For the same two Arabic msgids, the two English locales translate differently:
| Arabic msgid | en_US | en_MR |
|---|---|---|
رأس السنة الهجرية |
Islamic New Year | Muharram/Islamic New Year |
المولد النبوي الشريف |
The Prophet's Birthday | Prophet Muhammad's Birthday |
The en_US strings match the updated source comments in mauritania.py (# Islamic New Year., # The Prophet's Birthday.), while en_MR keeps the pre-change wording (Muharram/Islamic New Year, Mawlid al-Nabi -> Prophet Muhammad's Birthday). Is the difference intentional (some Mauritania-specific English usage), or should en_MR match the source/en_US wording? If there's no deliberate regional distinction, aligning them would avoid two English variants of the same holiday drifting apart.
2. .po header comments. Comparing with existing locales (e.g. en_US/LC_MESSAGES/DZ.po -> # Algeria holidays en_US localization.):
en_US/LC_MESSAGES/MR.pocurrently has# Mauritania holidays.- for a non-default locale this is usually# Mauritania holidays en_US localization.en_MR/LC_MESSAGES/MR.pohas# Mauritania holidays locale localization.- thelocalelooks like a leftover template placeholder that should readen_MR; there's also a doubled blank#line just below it.
The ar/LC_MESSAGES/MR.po header (# Mauritania holidays.) is correct as-is since ar is the default language.
None of this is blocking from my side - point 1 is the one I'd want a maintainer's read on. Deferring to @PPsyrius for the final call.
5 participants
Proposed change
Added Arabic localization for Mauritania (MR) holidays.
Changes
holidays/locale/ar/LC_MESSAGES/MR.poREADME.mdto include the new localizationRelated to #3152
Type of change
Checklist
make checklocally; all checks and tests passed.