Skip to content

feat: add Somalia (SO) country support with Arabic translations#3393

Closed
Prabhu-bit wants to merge 2 commits into
vacanza:devfrom
Prabhu-bit:add-somalia-arabic-translation
Closed

feat: add Somalia (SO) country support with Arabic translations#3393
Prabhu-bit wants to merge 2 commits into
vacanza:devfrom
Prabhu-bit:add-somalia-arabic-translation

Conversation

@Prabhu-bit

Copy link
Copy Markdown

This PR adds Arabic (ar) localization for Somalia (SO) holidays.

Localization: Added full Arabic translations in holidays/locale/ar/ for all Somalia public holidays.

Testing: Updated tests/countries/test_somalia.py to include localization tests for the Arabic locale.

Verification: All 19 tests (including the new Arabic assertion tests) pass locally.

[x]

Copilot AI review requested due to automatic review settings March 24, 2026 11:11
@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added Arabic language support for Somalia holidays (holiday names available in Arabic).
  • Chores

    • Improved package version resolution with a safer fallback when metadata isn't available.
  • Tests

    • Added tests covering Arabic localization for Somalia holidays.

Walkthrough

This PR adds Arabic localization to Somalia: declares supported_languages on the Somalia class, adds an Arabic PO catalog, updates version resolution logic to fall back to a repository VERSION file (with a safe default), and adds a test that asserts Arabic holiday names.

Changes

Cohort / File(s) Summary
Somalia country class
holidays/countries/somalia.py
Added supported_languages = ("ar", "en_US") class attribute and minor docstring/whitespace adjustments in __init__.
Arabic locale
holidays/locale/ar/LC_MESSAGES/SO.po
New Arabic gettext catalog with translations for Somalia holidays and the "%s (estimated)" template.
Version fallback
holidays/version.py
Replaced unconditional metadata lookup with a _get_version wrapper that tries importlib.metadata.version, falls back to reading a local VERSION file, and defaults to "0.0.0" on error.
Tests
tests/countries/test_somalia.py
Added test_l10n_ar to verify Arabic holiday names for specific Somalia dates.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
  • KJhellico
  • PPsyrius
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding Arabic language support for Somalia holidays.
Description check ✅ Passed The description is directly related to the changeset, detailing the Arabic localization work and testing updates.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Arabic (ar) localization support for Somalia (SO) holidays, with accompanying tests.

Changes:

  • Add Arabic locale resources for Somalia under holidays/locale/ar/LC_MESSAGES/.
  • Register Arabic as a supported language for the Somalia holiday entity.
  • Add Arabic localization assertions to Somalia’s country test suite.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
tests/countries/test_somalia.py Adds a new test validating Arabic holiday name localization.
holidays/version.py Changes how __version__ is defined.
holidays/locale/ar/LC_MESSAGES/SO.po Introduces Arabic translations for Somalia’s holiday strings.
holidays/locale/ar/LC_MESSAGES/SO.mo Adds a compiled gettext catalog for Somalia Arabic.
holidays/countries/somalia.py Declares Arabic as a supported language; adjusts docstring formatting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Website: https://github.com/vacanza/holidays
# License: MIT (see LICENSE file)
#
# United Arab Emirates holidays.

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header comment says "United Arab Emirates holidays." but this file is for Somalia (SO). Please correct the country name in the header to avoid misleading metadata in localization files.

Suggested change
# United Arab Emirates holidays.
# Somalia holidays.

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +48
#. New Year's Day.
msgid "New Year's Day"
msgstr "رأس السنة الميلادية"

#. Labour Day.
msgid "Labour Day"
msgstr "عيد العمال"

#. Independence Day.
msgid "Independence Day"
msgstr "عيد الاستقلال"

#. Republic Day.
msgid "Republic Day"
msgstr "عيد الجمهورية"

#. %s (estimated).
msgid "%s (estimated)"
msgstr "(المقدرة) %s" No newline at end of file

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PO file doesn’t include translations for several holiday names that Somalia defines (e.g., "Islamic New Year", "Ashura", "Prophet's Birthday", "Isra' and Mi'raj", "Eid al-Fitr", "Eid al-Adha"). As-is, Arabic localization will still show those holidays in English, which contradicts the PR description claiming full Arabic translations for all Somalia public holidays.

Copilot uses AI. Check for mistakes.
Comment thread holidays/countries/somalia.py Outdated
"""

country = "SO"
supported_languages = ("ar",)

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supported_languages only lists "ar". In this codebase, country entities that support Arabic typically include "en_US" as well (e.g., holidays/countries/bahrain.py sets supported_languages = ("ar", "en_US")). Omitting "en_US" can cause ICalExporter(show_language=True) to treat language="en_US" as unsupported and drop/raise unexpectedly. Consider including "en_US" alongside "ar".

Suggested change
supported_languages = ("ar",)
supported_languages = ("ar", "en_US")

Copilot uses AI. Check for mistakes.
Comment on lines +36 to +37
Whether to add "estimated" label to Islamic holidays name
if holiday date is estimated.

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docstring indentation under Args: is off: the description lines for islamic_show_estimated are not indented under the parameter name. This is inconsistent with other country classes and makes the rendered docs harder to read; indent the description block under islamic_show_estimated:.

Suggested change
Whether to add "estimated" label to Islamic holidays name
if holiday date is estimated.
Whether to add "estimated" label to Islamic holidays name
if holiday date is estimated.

Copilot uses AI. Check for mistakes.
Comment on lines +141 to +142

# Remove the extra space at the end of each Arabic string

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is trailing whitespace on the blank line after ar_holidays = Somalia(language="ar") (will trip Ruff/flake8 W291). Also, the comment about removing trailing spaces is misleading since the test doesn’t strip anything. Please remove the trailing whitespace and update/remove the comment to reflect what the test is actually asserting.

Suggested change
# Remove the extra space at the end of each Arabic string
# Ensure the Arabic holiday names match the expected strings exactly

Copilot uses AI. Check for mistakes.
Comment thread tests/countries/test_somalia.py Outdated
("2018-11-20", "Prophet's Birthday (estimated)"),
)

def test_l10n_arabic(self):

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test naming is inconsistent with the existing l10n tests in this repo, which use the locale code in the method name (e.g., test_l10n_ar, test_l10n_en_us). Consider renaming test_l10n_arabic to test_l10n_ar to match the established pattern in tests/countries/*.

Suggested change
def test_l10n_arabic(self):
def test_l10n_ar(self):

Copilot uses AI. Check for mistakes.
Comment thread holidays/version.py Outdated
from importlib.metadata import version

__version__ = version("holidays")
__version__ = "0.45" No newline at end of file

Copilot AI Mar 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__version__ is hard-coded to "0.45", but the repo’s packaging version is sourced from the VERSION file (currently 0.94). This will make holidays.__version__ disagree with distribution metadata and can break tests/test_package.py (which asserts metadata("holidays")["version"] == holidays.version). Restore a single source of truth for the version (e.g., use importlib.metadata.version("holidays") again or read from VERSION).

Suggested change
__version__ = "0.45"
from importlib.metadata import PackageNotFoundError, version as _get_version
from pathlib import Path
try:
__version__ = _get_version("holidays")
except PackageNotFoundError:
try:
_version_file = Path(__file__).resolve().parent.parent / "VERSION"
__version__ = _version_file.read_text(encoding="utf-8").strip()
except OSError:
# Fallback version if neither package metadata nor VERSION file is available.
__version__ = "0.0.0"

Copilot uses AI. Check for mistakes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
holidays/countries/somalia.py (1)

44-73: 🛠️ Refactor suggestion | 🟠 Major

Holiday names should use tr() wrapper for localization support.

With supported_languages declared, holiday names should be wrapped with tr() to enable proper localization. This is the established pattern for l10n-enabled country implementations.

Proposed fix
+from holidays.calendars.gregorian import JUN, JUL
+from holidays.groups import InternationalHolidays, IslamicHolidays
+from holidays.holiday_base import HolidayBase
+from holidays.l10n import tr

...

     def _populate_public_holidays(self):
         # New Year's Day.
-        self._add_new_years_day("New Year's Day")
+        self._add_new_years_day(tr("New Year's Day"))

         # Labor Day.
-        self._add_labor_day("Labour Day")
+        self._add_labor_day(tr("Labour Day"))

         # Independence Day.
-        self._add_holiday_jun_26("Independence Day")
+        self._add_holiday_jun_26(tr("Independence Day"))

         # Republic Day.
-        self._add_holiday_jul_1("Republic Day")
+        self._add_holiday_jul_1(tr("Republic Day"))

         # Islamic New Year.
-        self._add_islamic_new_year_day("Islamic New Year")
+        self._add_islamic_new_year_day(tr("Islamic New Year"))

         # Ashura.
-        self._add_ashura_day("Ashura")
+        self._add_ashura_day(tr("Ashura"))

         # Prophet's Birthday.
-        self._add_mawlid_day("Prophet's Birthday")
+        self._add_mawlid_day(tr("Prophet's Birthday"))

         # Isra' and Mi'raj.
-        self._add_isra_and_miraj_day("Isra' and Mi'raj")
+        self._add_isra_and_miraj_day(tr("Isra' and Mi'raj"))

         # Eid al-Fitr.
-        self._add_eid_al_fitr_day("Eid al-Fitr")
+        self._add_eid_al_fitr_day(tr("Eid al-Fitr"))

         # Eid al-Adha.
-        self._add_eid_al_adha_day("Eid al-Adha")
+        self._add_eid_al_adha_day(tr("Eid al-Adha"))

Based on learnings: "In the holidays library, countries with localization support DO use tr() wrappers around holiday names when calling add* methods."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/countries/somalia.py` around lines 44 - 73, The holiday name strings
in _populate_public_holidays should be wrapped with tr() for localization;
update each call to methods like _add_new_years_day, _add_labor_day,
_add_holiday_jun_26, _add_holiday_jul_1, _add_islamic_new_year_day,
_add_ashura_day, _add_mawlid_day, _add_isra_and_miraj_day, _add_eid_al_fitr_day,
and _add_eid_al_adha_day to pass tr("...") instead of raw literals (e.g.,
tr("New Year's Day")), ensuring the module’s supported_languages localization is
used consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@holidays/countries/somalia.py`:
- Around line 25-28: Add a default_language attribute set to "ar" and update
supported_languages to include "en_US" if you want consistency with other
Arabic-speaking countries; change supported_languages from ("ar",) to ("ar",
"en_US") (or keep only "ar" if you intentionally exclude English). Also wrap the
estimated_label in the translation helper by replacing the plain string with a
call to tr("%s (estimated)") so localization uses the .po files; adjust the
symbols country, supported_languages, default_language, and estimated_label
accordingly.

In `@holidays/locale/ar/LC_MESSAGES/SO.po`:
- Line 13: Replace the incorrect header comment "# United Arab Emirates
holidays." in the SO.po locale file with a correct description for Somalia
(e.g., "Somalia holidays ar localization.") so the header accurately reflects
the file's purpose; locate the offending comment line that currently reads "#
United Arab Emirates holidays." and update its text accordingly.
- Around line 46-48: The Arabic translation in the SO.po file uses the
placeholder after the text, which is inconsistent with the project pattern;
update the msgstr for the msgid "%s (estimated)" by moving the "%s" to the start
so it matches other Arabic locale files (see ET.po) — change the msgstr
"(المقدرة) %s" to "%s (المقدرة)" for the entry corresponding to msgid "%s
(estimated)".

In `@holidays/version.py`:
- Line 13: The __version__ constant in holidays/version.py is hardcoded as
"0.45" and must match the canonical version; update the __version__ variable in
holidays/version.py from "0.45" to "0.94" so it matches the VERSION file and
satisfies tests such as tests/test_package.py::TestPackage::test_metadata that
compare holidays.__version__ against the installed package version.

In `@tests/countries/test_somalia.py`:
- Around line 137-145: Remove the misleading inline comment in test_l10n_arabic
(there is no trailing space in the Arabic strings) and add an extra assertion
that verifies estimated Islamic holiday translations format correctly:
instantiate Somalia(language="ar") (same as ar_holidays) and assert that an
estimated Islamic holiday returned by ar_holidays.get(...) matches the "%s
(المقدرة)" pattern from the .po file (i.e., the holiday name followed by "
(المقدرة)" in Arabic); use an example date corresponding to an estimated Islamic
holiday from your test data so the assertion exercises the formatting path in
the Somalia.get method.

---

Outside diff comments:
In `@holidays/countries/somalia.py`:
- Around line 44-73: The holiday name strings in _populate_public_holidays
should be wrapped with tr() for localization; update each call to methods like
_add_new_years_day, _add_labor_day, _add_holiday_jun_26, _add_holiday_jul_1,
_add_islamic_new_year_day, _add_ashura_day, _add_mawlid_day,
_add_isra_and_miraj_day, _add_eid_al_fitr_day, and _add_eid_al_adha_day to pass
tr("...") instead of raw literals (e.g., tr("New Year's Day")), ensuring the
module’s supported_languages localization is used consistently.
🪄 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: d49bafd6-8de2-4d65-bfbb-777c7e3acd17

📥 Commits

Reviewing files that changed from the base of the PR and between bd01922 and 601a5a2.

📒 Files selected for processing (5)
  • holidays/countries/somalia.py
  • holidays/locale/ar/LC_MESSAGES/SO.mo
  • holidays/locale/ar/LC_MESSAGES/SO.po
  • holidays/version.py
  • tests/countries/test_somalia.py

Comment on lines 25 to 28
country = "SO"
supported_languages = ("ar",)
# %s (estimated).
estimated_label = "%s (estimated)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how other Arabic-supporting countries define language attributes
echo "=== Saudi Arabia ==="
grep -A2 "country = \"SA\"" holidays/countries/saudi_arabia.py

echo -e "\n=== UAE ==="
grep -A2 "country = \"AE\"" holidays/countries/united_arab_emirates.py

echo -e "\n=== Djibouti ==="
grep -A2 "country = \"DJ\"" holidays/countries/djibouti.py

Repository: vacanza/holidays

Length of output: 318


🏁 Script executed:

#!/bin/bash
# Get complete language configuration for Arabic-supporting countries
echo "=== Saudi Arabia (full lines 23-28) ==="
sed -n '23,28p' holidays/countries/saudi_arabia.py

echo -e "\n=== UAE (full lines 23-28) ==="
sed -n '23,28p' holidays/countries/united_arab_emirates.py

echo -e "\n=== Djibouti (full lines 23-28) ==="
sed -n '23,28p' holidays/countries/djibouti.py

echo -e "\n=== Somalia (full lines 23-30) ==="
sed -n '23,30p' holidays/countries/somalia.py

# Check for .po files
echo -e "\n=== Locale files for Somalia ==="
find holidays/locale -name "*SO*" -o -name "*so*" 2>/dev/null | head -20

echo -e "\n=== Locale files for Saudi Arabia ==="
find holidays/locale -name "*SA*" -o -name "*sa*" 2>/dev/null | head -20

Repository: vacanza/holidays

Length of output: 1580


🏁 Script executed:

#!/bin/bash
# Find default_language and supported_languages in Saudi Arabia
echo "=== Saudi Arabia language config ==="
grep -n "default_language\|supported_languages" holidays/countries/saudi_arabia.py | head -5

# Check Somalia's .po file for X-Source-Language
echo -e "\n=== Somalia .po file header (first 30 lines) ==="
head -30 holidays/locale/ar/LC_MESSAGES/SO.po

# Check if Somalia uses tr() in holiday definitions
echo -e "\n=== Somalia _populate_public_holidays tr() usage ==="
grep -A100 "def _populate_public_holidays" holidays/countries/somalia.py | grep -E "tr\(|self\._add"

Repository: vacanza/holidays

Length of output: 1729


🏁 Script executed:

#!/bin/bash
# Check the full header of Somalia .po file for X-Source-Language
echo "=== Somalia .po file full header ==="
head -50 holidays/locale/ar/LC_MESSAGES/SO.po | grep -E "^\"X-|^#"

# Also check Djibouti's .po for comparison
echo -e "\n=== Djibouti .po file header ==="
head -50 holidays/locale/ar/LC_MESSAGES/DJ.po | grep -E "^\"X-|^#"

# Check if Somalia's estimated_label needs to be using tr()
echo -e "\n=== Djibouti estimated_label pattern ==="
grep "estimated_label" holidays/countries/djibouti.py

# Verify Somalia doesn't have default_language set anywhere
echo -e "\n=== Somalia complete language config check ==="
grep -E "default_language|supported_languages|estimated_label" holidays/countries/somalia.py

Repository: vacanza/holidays

Length of output: 1760


🏁 Script executed:

#!/bin/bash
# Get the full msgstr section for Somalia .po to check for X-Source-Language
echo "=== Somalia .po file complete header msgstr ==="
sed -n '15,40p' holidays/locale/ar/LC_MESSAGES/SO.po

# Compare with a working example like Saudi Arabia
echo -e "\n=== Saudi Arabia .po file header section ==="
sed -n '15,40p' holidays/locale/ar/LC_MESSAGES/SA.po

# Check the actual holiday entries in Somalia to see tr() pattern needed
echo -e "\n=== Somalia first translated entries ==="
sed -n '30,60p' holidays/locale/ar/LC_MESSAGES/SO.po

Repository: vacanza/holidays

Length of output: 1700


Add default_language attribute and update localization setup.

Somalia has .po translation files but is missing the required default_language attribute. This should match the primary language of the translations. Compare with Saudi Arabia's pattern:

default_language = "ar"
supported_languages = ("ar", "en_US")
estimated_label = tr("%s (estimated)")

Somalia should also use tr() wrapper on estimated_label to enable proper localization, and consider whether to include "en_US" in supported_languages for consistency with other Arabic-speaking countries.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/countries/somalia.py` around lines 25 - 28, Add a default_language
attribute set to "ar" and update supported_languages to include "en_US" if you
want consistency with other Arabic-speaking countries; change
supported_languages from ("ar",) to ("ar", "en_US") (or keep only "ar" if you
intentionally exclude English). Also wrap the estimated_label in the translation
helper by replacing the plain string with a call to tr("%s (estimated)") so
localization uses the .po files; adjust the symbols country,
supported_languages, default_language, and estimated_label accordingly.

# Website: https://github.com/vacanza/holidays
# License: MIT (see LICENSE file)
#
# United Arab Emirates holidays.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Incorrect header comment - copy-paste error.

The comment says "United Arab Emirates holidays" but this file is for Somalia. Should be "Somalia holidays ar localization."

Proposed fix
-# United Arab Emirates holidays.
+# Somalia holidays ar localization.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# United Arab Emirates holidays.
# Somalia holidays ar localization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/locale/ar/LC_MESSAGES/SO.po` at line 13, Replace the incorrect
header comment "# United Arab Emirates holidays." in the SO.po locale file with
a correct description for Somalia (e.g., "Somalia holidays ar localization.") so
the header accurately reflects the file's purpose; locate the offending comment
line that currently reads "# United Arab Emirates holidays." and update its text
accordingly.

Comment on lines +46 to +48
#. %s (estimated).
msgid "%s (estimated)"
msgstr "(المقدرة) %s" No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Format specifier position inconsistent with codebase pattern.

The %s placeholder should be at the start of the string to match the established pattern in other Arabic locale files. See holidays/locale/ar/LC_MESSAGES/ET.po which uses msgstr "%s (المقدرة)".

Proposed fix
 #. %s (estimated).
 msgid "%s (estimated)"
-msgstr "(المقدرة) %s"
+msgstr "%s (المقدرة)"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#. %s (estimated).
msgid "%s (estimated)"
msgstr "(المقدرة) %s"
#. %s (estimated).
msgid "%s (estimated)"
msgstr "%s (المقدرة)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/locale/ar/LC_MESSAGES/SO.po` around lines 46 - 48, The Arabic
translation in the SO.po file uses the placeholder after the text, which is
inconsistent with the project pattern; update the msgstr for the msgid "%s
(estimated)" by moving the "%s" to the start so it matches other Arabic locale
files (see ET.po) — change the msgstr "(المقدرة) %s" to "%s (المقدرة)" for the
entry corresponding to msgid "%s (estimated)".

Comment thread holidays/version.py Outdated
Comment thread tests/countries/test_somalia.py Outdated
Comment on lines +137 to +145
def test_l10n_arabic(self):
"""Test if Arabic translations from SO.po are correctly loaded."""
# Initialize Somalia class with Arabic language
ar_holidays = Somalia(language="ar")

# Remove the extra space at the end of each Arabic string
self.assertEqual(ar_holidays.get("2026-06-26"), "عيد الاستقلال")
self.assertEqual(ar_holidays.get("2026-07-01"), "عيد الجمهورية")
self.assertEqual(ar_holidays.get("2026-05-01"), "عيد العمال")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Test structure looks good; consider expanding coverage.

The unittest-style assertEqual is correct per project conventions - ignore the Ruff PT009 warnings.

A couple of notes:

  1. The comment on line 142 ("Remove the extra space...") is confusing since there's no extra space in the Arabic strings. Consider removing this misleading comment.
  2. Consider adding a test for estimated Islamic holidays to verify the "%s (المقدرة)" translation works correctly, especially given the format specifier concern in the .po file.
Proposed improvements
     def test_l10n_arabic(self):
         """Test if Arabic translations from SO.po are correctly loaded."""
-        # Initialize Somalia class with Arabic language
         ar_holidays = Somalia(language="ar")
-        
-        # Remove the extra space at the end of each Arabic string
         self.assertEqual(ar_holidays.get("2026-06-26"), "عيد الاستقلال")
         self.assertEqual(ar_holidays.get("2026-07-01"), "عيد الجمهورية")
         self.assertEqual(ar_holidays.get("2026-05-01"), "عيد العمال")
+        
+        # Test estimated label translation for Islamic holidays
+        ar_holidays_with_estimated = Somalia(years=2018, language="ar")
+        # Verify estimated Islamic holiday includes translated label
+        self.assertIn("المقدرة", ar_holidays_with_estimated.get("2018-06-15", ""))
🧰 Tools
🪛 Ruff (0.15.6)

[warning] 143-143: Use a regular assert instead of unittest-style assertEqual

Replace assertEqual(...) with assert ...

(PT009)


[warning] 144-144: Use a regular assert instead of unittest-style assertEqual

Replace assertEqual(...) with assert ...

(PT009)


[warning] 145-145: Use a regular assert instead of unittest-style assertEqual

Replace assertEqual(...) with assert ...

(PT009)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/countries/test_somalia.py` around lines 137 - 145, Remove the
misleading inline comment in test_l10n_arabic (there is no trailing space in the
Arabic strings) and add an extra assertion that verifies estimated Islamic
holiday translations format correctly: instantiate Somalia(language="ar") (same
as ar_holidays) and assert that an estimated Islamic holiday returned by
ar_holidays.get(...) matches the "%s (المقدرة)" pattern from the .po file (i.e.,
the holiday name followed by " (المقدرة)" in Arabic); use an example date
corresponding to an estimated Islamic holiday from your test data so the
assertion exercises the formatting path in the Somalia.get method.

@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (4)
holidays/locale/ar/LC_MESSAGES/SO.po (2)

66-68: ⚠️ Potential issue | 🟠 Major

Format specifier position inconsistent with codebase pattern.

The %s placeholder should be at the start of the string to match the established pattern in other Arabic locale files like holidays/locale/ar/LC_MESSAGES/ET.po which uses msgstr "%s (المقدرة)".

Proposed fix
 #. %s (estimated).
 msgid "%s (estimated)"
-msgstr "(المقدرة) %s"
+msgstr "%s (المقدرة)"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/locale/ar/LC_MESSAGES/SO.po` around lines 66 - 68, The Arabic
translation for the "%s (estimated)" message has the placeholder at the end
which is inconsistent; update the msgstr for the msgid "%s (estimated)" so the
"%s" placeholder appears at the start (matching the pattern used in other Arabic
locale files), i.e. change the msgstr "(المقدرة) %s" to place "%s" first and
keep the Arabic text after it.

13-13: ⚠️ Potential issue | 🟡 Minor

Incorrect header comment - copy-paste error.

The comment says "United Arab Emirates holidays" but this file is for Somalia. Should be "Somalia holidays ar localization."

Proposed fix
-# United Arab Emirates holidays.
+# Somalia holidays ar localization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/locale/ar/LC_MESSAGES/SO.po` at line 13, Replace the incorrect
header comment "# United Arab Emirates holidays." in the SO.po file with a
correct Somalia-specific comment such as "Somalia holidays ar localization." so
the header accurately reflects the file's locale and purpose (identify the line
containing "# United Arab Emirates holidays." to update).
holidays/countries/somalia.py (1)

25-28: ⚠️ Potential issue | 🟠 Major

Add default_language attribute and wrap estimated_label with tr().

Somalia now has .po translation files but is missing the default_language attribute and tr() wrapper. Compare with similar Arabic-speaking countries like Saudi Arabia or Djibouti:

default_language = "ar"
supported_languages = ("ar", "en_US")
# %s (estimated).
estimated_label = tr("%s (estimated)")

Without default_language, the library won't know which language to use by default. The tr() wrapper is required for the estimated label to be translated via the .po file.

Proposed fix
 country = "SO"
+default_language = "ar"
 supported_languages = ("ar", "en_US")
 # %s (estimated).
-estimated_label = "%s (estimated)"
+estimated_label = tr("%s (estimated)")

Also add the import at the top:

from gettext import gettext as tr
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/countries/somalia.py` around lines 25 - 28, Add a default_language
attribute and make the estimated label translatable: set default_language = "ar"
(to match other Arabic-speaking country modules), wrap estimated_label using
tr() so it reads tr("%s (estimated)"), and ensure the module imports gettext as
tr (from gettext import gettext as tr) near the top; update the country/Somalia
module attributes country, supported_languages, default_language, and
estimated_label accordingly.
tests/countries/test_somalia.py (1)

137-145: 🧹 Nitpick | 🔵 Trivial

Misleading comment and limited test coverage.

The comment on line 142 ("Remove the extra space...") is misleading since there's no extra space removal happening. The test also only covers 3 holidays. Consider:

  1. Remove the misleading comment
  2. Test New Year's Day Arabic translation
  3. Test at least one Islamic holiday Arabic translation
  4. Test the estimated label format (e.g., "عيد الفطر (المقدرة)" for an estimated date)
Proposed improvements
 def test_l10n_ar(self):
-    """Test if Arabic translations from SO.po are correctly loaded."""
-    # Initialize Somalia class with Arabic language
     ar_holidays = Somalia(language="ar")
-    
-    # Remove the extra space at the end of each Arabic string
+    self.assertEqual(ar_holidays.get("2026-01-01"), "رأس السنة الميلادية")
     self.assertEqual(ar_holidays.get("2026-06-26"), "عيد الاستقلال")
     self.assertEqual(ar_holidays.get("2026-07-01"), "عيد الجمهورية")
     self.assertEqual(ar_holidays.get("2026-05-01"), "عيد العمال")
+    # Test Islamic holiday
+    self.assertEqual(ar_holidays.get("2026-06-06"), "عيد الأضحى")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/countries/test_somalia.py` around lines 137 - 145, The test_l10n_ar in
tests/countries/test_somalia.py contains a misleading comment and insufficient
coverage; remove the comment about "Remove the extra space..." and extend the
test by asserting Arabic translations via Somalia(language="ar") for New Year's
Day (use ar_holidays.get("YYYY-01-01") with the expected Arabic label), at least
one Islamic holiday (e.g., check ar_holidays.get for an Eid date), and verify an
estimated label format by asserting an estimated holiday returns the form like
"عيد الفطر (المقدرة)" using ar_holidays.get on an estimated-date key; update
only the test_l10n_ar function and use ar_holidays.get(...) calls to add these
assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@holidays/locale/ar/LC_MESSAGES/SO.po`:
- Around line 58-65: Add the missing Arabic translation for the "Isra' and
Mi'raj" holiday referenced in somalia.py by adding a msgid/msgstr pair for
"Isra' and Mi'raj" to the PO file (place it after the "Eid al-Adha" entry to
match the locale order); use the Arabic translation "الإسراء والمعراج" for
msgstr so the msgid "Isra' and Mi'raj" has a proper localized string.

---

Duplicate comments:
In `@holidays/countries/somalia.py`:
- Around line 25-28: Add a default_language attribute and make the estimated
label translatable: set default_language = "ar" (to match other Arabic-speaking
country modules), wrap estimated_label using tr() so it reads tr("%s
(estimated)"), and ensure the module imports gettext as tr (from gettext import
gettext as tr) near the top; update the country/Somalia module attributes
country, supported_languages, default_language, and estimated_label accordingly.

In `@holidays/locale/ar/LC_MESSAGES/SO.po`:
- Around line 66-68: The Arabic translation for the "%s (estimated)" message has
the placeholder at the end which is inconsistent; update the msgstr for the
msgid "%s (estimated)" so the "%s" placeholder appears at the start (matching
the pattern used in other Arabic locale files), i.e. change the msgstr
"(المقدرة) %s" to place "%s" first and keep the Arabic text after it.
- Line 13: Replace the incorrect header comment "# United Arab Emirates
holidays." in the SO.po file with a correct Somalia-specific comment such as
"Somalia holidays ar localization." so the header accurately reflects the file's
locale and purpose (identify the line containing "# United Arab Emirates
holidays." to update).

In `@tests/countries/test_somalia.py`:
- Around line 137-145: The test_l10n_ar in tests/countries/test_somalia.py
contains a misleading comment and insufficient coverage; remove the comment
about "Remove the extra space..." and extend the test by asserting Arabic
translations via Somalia(language="ar") for New Year's Day (use
ar_holidays.get("YYYY-01-01") with the expected Arabic label), at least one
Islamic holiday (e.g., check ar_holidays.get for an Eid date), and verify an
estimated label format by asserting an estimated holiday returns the form like
"عيد الفطر (المقدرة)" using ar_holidays.get on an estimated-date key; update
only the test_l10n_ar function and use ar_holidays.get(...) calls to add these
assertions.
🪄 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: 2678a6f4-466a-4e37-bc3c-ffd9fd007765

📥 Commits

Reviewing files that changed from the base of the PR and between 601a5a2 and 990b744.

📒 Files selected for processing (5)
  • holidays/countries/somalia.py
  • holidays/locale/ar/LC_MESSAGES/SO.mo
  • holidays/locale/ar/LC_MESSAGES/SO.po
  • holidays/version.py
  • tests/countries/test_somalia.py

Comment on lines +58 to +65
#. Eid al-Fitr.
msgid "Eid al-Fitr"
msgstr "عيد الفطر"

#. Eid al-Adha.
msgid "Eid al-Adha"
msgstr "عيد الأضحى"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing translation for "Isra' and Mi'raj".

The Somalia implementation in somalia.py includes Isra' and Mi'raj holiday (Line 67), but this translation is missing from the PO file. Add the Arabic translation for completeness.

Proposed fix - add after Eid al-Adha entry
 #. Eid al-Adha.
 msgid "Eid al-Adha"
 msgstr "عيد الأضحى"
+
+#. Isra' and Mi'raj.
+msgid "Isra' and Mi'raj"
+msgstr "الإسراء والمعراج"

 #. %s (estimated).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@holidays/locale/ar/LC_MESSAGES/SO.po` around lines 58 - 65, Add the missing
Arabic translation for the "Isra' and Mi'raj" holiday referenced in somalia.py
by adding a msgid/msgstr pair for "Isra' and Mi'raj" to the PO file (place it
after the "Eid al-Adha" entry to match the locale order); use the Arabic
translation "الإسراء والمعراج" for msgstr so the msgid "Isra' and Mi'raj" has a
proper localized string.

@KJhellico KJhellico marked this pull request as draft March 24, 2026 15:31
@KJhellico

Copy link
Copy Markdown
Collaborator

@Prabhu-bit, are you planning to finish this PR?

@Prabhu-bit

Copy link
Copy Markdown
Author

Just I wanted to contribute as a beginner , thanks for reviews

@KJhellico

Copy link
Copy Markdown
Collaborator

As I understand it, that's "no" answer. 🤷‍♂️

@KJhellico KJhellico closed this Apr 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants