-
-
Notifications
You must be signed in to change notification settings - Fork 539
Update Namibia holidays, add l10n support #2629
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 internationalization and localization support for Namibia's holidays, adds and restructures special holiday definitions, and expands test coverage. It provides new translation files for English (Namibia and US) and Ukrainian, updates the Namibia holiday logic, enhances documentation, and improves test granularity and localization checks. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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 #2629 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 230 230
Lines 14636 14643 +7
Branches 2045 2046 +1
=========================================
+ Hits 14636 14643 +7 ☔ 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
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.gitignore
(1 hunks)README.md
(1 hunks)holidays/countries/namibia.py
(3 hunks)holidays/locale/en_NA/LC_MESSAGES/NA.po
(1 hunks)holidays/locale/en_US/LC_MESSAGES/NA.po
(1 hunks)holidays/locale/uk/LC_MESSAGES/NA.po
(1 hunks)snapshots/countries/NA_COMMON.json
(38 hunks)tests/countries/test_namibia.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
holidays/locale/en_US/LC_MESSAGES/NA.po (1)
Learnt from: KJhellico
PR: vacanza/holidays#2388
File: holidays/locale/en_CI/LC_MESSAGES/CI.po:88-88
Timestamp: 2025-03-30T18:25:07.087Z
Learning: In the holidays library, localization files have a specific structure: message comments are in standard English (en_US) describing the holiday, while actual translations (msgstr) should use the locale-specific terminology (e.g., en_CI for Ivory Coast English). For example, "Night of Power" in standard English is translated as "Lailatou-Kadr" in Ivory Coast English.
tests/countries/test_namibia.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
🧬 Code Graph Analysis (2)
holidays/countries/namibia.py (3)
holidays/groups/christian.py (6)
ChristianHolidays
(22-463)_add_good_friday
(308-317)_add_easter_monday
(259-268)_add_ascension_thursday
(119-127)_add_christmas_day
(208-216)_add_christmas_day_two
(218-226)holidays/groups/international.py (4)
InternationalHolidays
(18-220)_add_new_years_day
(126-134)_add_labor_day
(99-108)_add_africa_day
(30-39)holidays/observed_holiday_base.py (1)
_add_observed
(143-193)
tests/countries/test_namibia.py (2)
holidays/countries/namibia.py (2)
Namibia
(20-94)NA
(97-98)tests/common.py (7)
assertAliases
(121-130)assertHoliday
(150-152)assertHolidayName
(195-199)assertNoNonObservedHoliday
(248-250)assertNoHolidayName
(273-275)assertHolidays
(228-230)assertLocalizedHolidays
(327-338)
🪛 Pylint (3.3.7)
holidays/countries/namibia.py
[convention] 24-24: Line too long (136/100)
(C0301)
[convention] 25-25: Line too long (146/100)
(C0301)
[convention] 26-26: Line too long (174/100)
(C0301)
[convention] 109-109: Line too long (162/100)
(C0301)
[convention] 110-110: Line too long (165/100)
(C0301)
[convention] 111-111: Line too long (166/100)
(C0301)
[convention] 112-112: Line too long (168/100)
(C0301)
[convention] 113-113: Line too long (167/100)
(C0301)
[convention] 114-114: Line too long (167/100)
(C0301)
[convention] 115-115: Line too long (164/100)
(C0301)
[convention] 116-116: Line too long (167/100)
(C0301)
[convention] 117-117: Line too long (167/100)
(C0301)
[convention] 118-118: Line too long (167/100)
(C0301)
[convention] 119-119: Line too long (167/100)
(C0301)
[convention] 120-120: Line too long (167/100)
(C0301)
[convention] 121-121: Line too long (167/100)
(C0301)
[convention] 122-122: Line too long (163/100)
(C0301)
tests/countries/test_namibia.py
[convention] 25-25: Missing function or method docstring
(C0116)
[convention] 28-28: Missing function or method docstring
(C0116)
[convention] 31-31: Missing function or method docstring
(C0116)
[convention] 50-50: Missing function or method docstring
(C0116)
[convention] 63-63: Missing function or method docstring
(C0116)
[convention] 76-76: Missing function or method docstring
(C0116)
[convention] 89-89: Missing function or method docstring
(C0116)
[convention] 102-102: Missing function or method docstring
(C0116)
[convention] 115-115: Missing function or method docstring
(C0116)
[convention] 128-128: Missing function or method docstring
(C0116)
[convention] 141-141: Missing function or method docstring
(C0116)
[convention] 146-146: Missing function or method docstring
(C0116)
[convention] 159-159: Missing function or method docstring
(C0116)
[convention] 171-171: Missing function or method docstring
(C0116)
[convention] 193-193: Missing function or method docstring
(C0116)
[convention] 196-196: Missing function or method docstring
(C0116)
[convention] 209-209: Missing function or method docstring
(C0116)
[convention] 228-228: Missing function or method docstring
(C0116)
[convention] 248-248: Missing function or method docstring
(C0116)
[convention] 269-269: Missing function or method docstring
(C0116)
⏰ Context from checks skipped due to timeout of 300000ms (3)
- GitHub Check: Test build on windows-latest
- GitHub Check: Test build on macos-latest
- GitHub Check: Test build on ubuntu-latest
🔇 Additional comments (5)
.gitignore (1)
9-9
: 👍 Good call ignoring CSV artifactsHelps avoid accidental commits of generated data.
README.md (1)
962-966
: README row looks goodLanguages list matches code (
default_language="en_NA"
).holidays/locale/uk/LC_MESSAGES/NA.po (1)
29-33
: Place-holder preserved correctly
%s
token kept, avoids runtimeKeyError
.snapshots/countries/NA_COMMON.json (1)
40-53
: Snapshot entries align with codeNew special holidays and Genocide Remembrance Day appear in expected years.
Also applies to: 320-335, 333-335, 385-387, 398-400, 443-458, 456-467
holidays/locale/en_NA/LC_MESSAGES/NA.po (1)
29-120
: 🧹 Nitpick (assertive)
⚠️ Potential issueEmpty translations break default locale ‑ fill
msgstr
Every
msgstr
is""
, sogettext("…")
returns an empty string for the defaulten_NA
locale. All holiday names will come back blank, causing display issues and test failures.Patch idea (apply to all entries, shown for one):
-msgid "New Year's Day" -msgstr "" +msgid "New Year's Day" +msgstr "New Year's Day"A quick fix:
# autofill all empty msgstr with msgid msgcat --no-wrap --force-po --inverse --output - NA.po | msgmerge - NA.po | sponge NA.poUntil proper Namibian-English terminology is provided, copying
msgid
intomsgstr
preserves functionality.⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library, when a locale file matches the country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language files should have filled msgstr fields with translations.
Learnt from: KJhellico PR: vacanza/holidays#2259 File: holidays/locale/en_IN/LC_MESSAGES/IN.po:30-299 Timestamp: 2025-03-05T17:51:00.633Z Learning: In the Holidays project, .po files for a country's default locale use empty msgstr fields as a standard convention.
Learnt from: KJhellico PR: vacanza/holidays#2394 File: holidays/locale/pt_PT/LC_MESSAGES/CV.po:31-88 Timestamp: 2025-03-31T19:37:57.691Z Learning: In the holidays library localization pattern, when a locale file matches a country's default language (e.g., pt_PT for Cape Verde), the msgstr fields should be left empty. Only non-default language locale files should have translations in the msgstr fields.
Learnt from: PPsyrius PR: vacanza/holidays#2438 File: holidays/locale/ar_IQ/LC_MESSAGES/IQ.po:35-81 Timestamp: 2025-04-17T17:08:48.082Z Learning: In holiday PO files, when the file represents the default language of an entity (e.g., ar_IQ for Iraq), no translations in `msgstr` are required as the `msgid` values are already in the target language.
Learnt from: PPsyrius PR: vacanza/holidays#2388 File: holidays/locale/fr/LC_MESSAGES/CI.po:28-101 Timestamp: 2025-03-30T13:33:31.598Z Learning: In the holidays library, for localization files of the default language (like French for Ivory Coast in fr/LC_MESSAGES/CI.po), the best practice is to leave the message strings (msgstr) empty to avoid possible typos, since the message IDs (msgid) are already in the target language.
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 👍
Proposed change
Update Namibia holidays, add l10n support (en_NA, en_US, uk).
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green