Add Nauru holidays #2609
Add Nauru holidays #2609
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes introduce official support for Nauru holidays by adding a new country module, updating the registry, providing localization, and including comprehensive tests. Documentation and country lists are updated to reflect the new support. No existing logic is modified outside of adding Nauru. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (4)
✨ 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 #2609 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 230 231 +1
Lines 14636 14681 +45
Branches 2045 2050 +5
=========================================
+ Hits 14636 14681 +45 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Nauru is a small island so there are not many sources about its holidays. The inhabitants speak Nauruan and English (language of government and commerce). Although Nauruan is spoken by almost 96% people, I couldn't find any sources that has holidays mentioned in Nauruan. Also Google translate doesn't support Nauruan. This states that: "It became a German Colony in the late 19th century but was later administered by Australia". So, I have used en_AU. (Please let me know if that needs to be changed) |
|
Public Service Act 1998 (37 - Holidays) |
According to Public Service Act 2016, should I implement Independence Day like this: as it is mentioned that "there is no holiday on the day and instead the following Monday and Tuesday are holidays in the public service" for Independence Day and should I implement similar logic for all the holidays? And should I remove International Women's Day, RONPhos Handover Day, Ibumin Earoeni Day, Sir Hammer DeRoburt Day (with their references) as these are not mentioned in the official documents? Also, what should we name the holiday on Feb 1? |
self._add_observed(
# Independence Day.
self._add_holiday_jan_31(tr("Independence Day")),
rule=SAT_SUN_TO_NEXT_MON_TUE,
)
self._add_observed(
# Day following Independence Day.
self._add_holiday_feb_1(tr("Day following Independence Day")),
rule=SAT_SUN_TO_NEXT_MON_TUE,
) |
Obviously, these holidays were introduced later. Or maybe they are declared annually. |
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 (4)
README.md(2 hunks)holidays/countries/nauru.py(1 hunks)holidays/locale/en_AU/LC_MESSAGES/NR.po(1 hunks)tests/c 8000 ountries/test_nauru.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_nauru.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.
🪛 Pylint (3.3.7)
holidays/countries/nauru.py
[convention] 34-34: Line too long (141/100)
(C0301)
[convention] 38-38: Line too long (187/100)
(C0301)
[convention] 39-39: Line too long (172/100)
(C0301)
[convention] 40-40: Line too long (164/100)
(C0301)
[convention] 41-41: Line too long (164/100)
(C0301)
[convention] 42-42: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 149-149: Missing class docstring
(C0115)
[convention] 153-153: Missing class docstring
(C0115)
tests/countries/test_nauru.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestNauru.setUpClass' method
(W0221)
[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] 47-47: Missing function or method docstring
(C0116)
[convention] 61-61: Missing function or method docstring
(C0116)
[convention] 75-75: Missing function or method docstring
(C0116)
[convention] 86-86: Missing function or method docstring
(C0116)
[convention] 99-99: Missing function or method docstring
(C0116)
[convention] 112-112: Missing function or method docstring
(C0116)
[convention] 125-125: Missing function or method docstring
(C0116)
[convention] 149-149: Missing function or method docstring
(C0116)
[convention] 169-169: Missing function or method docstring
(C0116)
[convention] 186-186: Missing function or method docstring
(C0116)
[convention] 205-205: Missing function or method docstring
(C0116)
[convention] 230-230: Missing function or method docstring
(C0116)
[convention] 254-254: Missing function or method docstring
(C0116)
[convention] 277-277: Missing function or method docstring
(C0116)
[convention] 297-297: Missing function or method docstring
(C0116)
[convention] 320-320: Missing function or method docstring
(C0116)
🔇 Additional comments (5)
README.md (2)
108-108: Documentation update looks good.The country count increment is accurate and reflects the addition of Nauru support.
969-974: Nauru entry follows the established pattern.The table entry correctly shows NR code, no subdivisions, en_AU as default language, and no additional categories. This aligns with the implementation and locale choice discussed in the PR.
holidays/locale/en_AU/LC_MESSAGES/NR.po (1)
1-90: Localization file is well-structured.The .po file contains all necessary holiday entries with proper headers. Empty msgstr entries are correct for en_AU locale since the source text is already in Australian English.
holidays/countries/nauru.py (1)
24-147: Solid implementation following established patterns.The holiday definitions are comprehensive and use appropriate observance rules. The use of
self._add_observed()with proper rules likeSAT_SUN_TO_NEXT_MON_TUEfor Independence Day and Christmas aligns with the official requirements discussed in the PR.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 34-34: Line too long (141/100)
(C0301)
[convention] 38-38: Line too long (187/100)
(C0301)
[convention] 39-39: Line too long (172/100)
(C0301)
[convention] 40-40: Line too long (164/100)
(C0301)
[convention] 41-41: Line too long (164/100)
(C0301)
[convention] 42-42: Line too long (172/100)
(C0301)
tests/countries/test_nauru.py (1)
19-341: Comprehensive test suite with excellent coverage.The tests properly verify all holidays, observance rules, introduction years, aliases, and localization. The structure follows project conventions by testing individual holidays across multiple years. The explicit date testing provides clear verification of observed holiday logic.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 19-19: Missing class docstring
(C0115)
[warning] 21-21: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestNauru.setUpClass' method
(W0221)
[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] 47-47: Missing function or method docstring
(C0116)
[convention] 61-61: Missing function or method docstring
(C0116)
[convention] 75-75: Missing function or method docstring
(C0116)
[convention] 86-86: Missing function or method docstring
(C0116)
[convention] 99-99: Missing function or method docstring
(C0116)
[convention] 112-112: Missing function or method docstring
(C0116)
[convention] 125-125: Missing function or method docstring
(C0116)
[convention] 149-149: Missing function or method docstring
(C0116)
[convention] 169-169: Missing function or method docstring
(C0116)
[convention] 186-186: Missing function or method docstring
(C0116)
[convention] 205-205: Missing function or method docstring
(C0116)
[convention] 230-230: Missing function or method docstring
(C0116)
[convention] 254-254: Missing function or method docstring
(C0116)
[convention] 277-277: Missing function or method docstring
(C0116)
[convention] 297-297: Missing function or method docstring
(C0116)
[convention] 320-320: Missing function or method docstring
(C0116)
@KJhellico @PPsyrius However, I see some inconsistencies: |
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 (3)
README.md(1 hunks)holidays/countries/__init__.py(1 hunks)holidays/registry.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (1)
holidays/countries/nauru.py (3)
Nauru(24-146)NR(149-150)NRU(153-154)
🔇 Additional comments (2)
holidays/countries/__init__.py (1)
137-138: Import looks good and keeps the file alphabetised
Nauru, NR, NRUslot in neatly after Namibia and before Nepal; no naming collisions detected.
Nothing else to flag here.holidays/registry.py (1)
139-141: Registry entry checks outKey, class name and ISO-codes are all correct, and the alphabetical ordering is preserved.
All good.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
README.md(2 hunks)holidays/countries/nauru.py(1 hunks)holidays/locale/en_US/LC_MESSAGES/NR.po(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/nauru.py (2)
Learnt from: KJhellico
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:149-154
Timestamp: 2025-06-09T19:50:56.039Z
Learning: In the holidays library project, never suggest adding docstrings for alias classes (like NR, NRU country code aliases that inherit from main country classes). The project deliberately omits docstrings for these simple alias classes.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🪛 Pylint (3.3.7)
holidays/countries/nauru.py
[convention] 28-28: Line too long (120/100)
(C0301)
[convention] 29-29: Line too long (108/100)
(C0301)
[convention] 33-33: Line too long (176/100)
(C0301)
[convention] 34-34: Line too long (176/100)
(C0301)
[convention] 35-35: Line too long (172/100)
(C0301)
[convention] 36-36: Line too long (133/100)
(C0301)
[convention] 37-37: Line too long (153/100)
(C0301)
[convention] 38-38: Line too long (134/100)
(C0301)
[convention] 39-39: Line too long (134/100)
(C0301)
[convention] 40-40: Line too long (142/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 124-124: Missing class docstring
(C0115)
[convention] 128-128: Missing class docstring
(C0115)
🔇 Additional comments (2)
README.md (1)
106-110: Paragraph update looks goodCountry count bumped to 187; change aligns with table entry and previous feedback.
🧰 Tools
🪛 LanguageTool
[typographical] ~110-~110: Consider adding a comma after this introductory phrase.
Context: ...e same used for domain names, and for a subdivision its [ISO 3166-2 code](https://en.wikipe...(AS_A_NN_COMMA)
holidays/locale/en_US/LC_MESSAGES/NR.po (1)
23-34: Translation catalogue complete and consistentAll message IDs required by
nauru.pyare present; naming matches code exactly.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Roshan Pradhan <roshanpradhan1024@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
holidays/countries/nauru.py (2)
47-48: 🧹 Nitpick (assertive)Consider keeping
en_AUinsupported_languagesEarly discussion mentioned
en_AUas a viable locale; removing it may break sample code or documentation written before the final decision.- supported_languages = ("en_NR", "en_US") + supported_languages = ("en_NR", "en_US", "en_AU")Up to you—this is a non-breaking, nice-to-have addition.
61-71: 💡 Verification agent🧩 Analysis chain
Please re-check Monday/Tuesday ordering for the Independence-Day pair
Using
SAT_SUN_TO_NEXT_MON_TUEfor both 31 Jan and 1 Feb yields
• Saturday-case → Mon = Independence (OK), Tue = Day-after (OK)
• Sunday-case → Mon = Day-after, Tue = Independence (reversed)If the Public Service Act requires Independence Day to be the first working day (Monday) and the “day following” the second (Tuesday), the current rule set is off by one day whenever 31 Jan is a Sunday.
If the output shows Monday → “Day following …” and Tuesday → “Independence Day (observed)”, the order is inverted.
Adjust the rules (e.g.MONfor Independence /MON_TUEfor the following day) if necessary.
🏁 Script executed:
#!/bin/bash # Inspect 2021 where 31 Jan was Sunday. python - <<'PY' import holidays, datetime as dt yr = 2021 for d, n in sorted(holidays.Nauru(years=yr).items()): if d.month in (1,2): print(d, d.strftime("%A"), n) PYLength of output: 174
Re-shift Independence Day to the first working day
Running 2021 through the current rules shows:2021-01-31 (Sun) Independence Day 2021-02-01 (Mon) Day following Independence Day 2021-02-02 (Tue) Independence Day (observed)That flips the two when 31 Jan is Sunday. To guarantee Jan 31 always lands on the first working day (Mon) and Feb 1 on the second (Tue), split the rules:
holidays/countries/nauru.py (around lines 61–71):
- self._add_observed( - # Independence Day. - self._add_holiday_jan_31(tr("Independence Day")), - rule=SAT_SUN_TO_NEXT_MON_TUE, - ) + self._add_observed( + # Independence Day – always shift weekend to Monday + self._add_holiday_jan_31(tr("Independence Day")), + rule=MON, + ) - self._add_observed( - # Day following Independence Day. - self._add_holiday_feb_1(tr("Day following Independence Day")), - rule=SAT_SUN_TO_NEXT_MON_TUE, - ) + self._add_observed( + # Day following – shift weekend→Mon/Tue and bump any Monday collision to Tuesday + self._add_holiday_feb_1(tr("Day following Independence Day")), + rule=MON_TUE, + )With this change, 2021 becomes
• Mon 2021-02-01: Independence Day (observed)
• Tue 2021-02-02: Day following Independence Day (observed)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/nauru.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/nauru.py (2)
Learnt from: KJhellico
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:149-154
Timestamp: 2025-06-09T19:50:56.039Z
Learning: In the holidays library project, never suggest adding docstrings for alias classes (like NR, NRU country code aliases that inherit from main country classes). The project deliberately omits docstrings for these simple alias classes.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-23T10:11:50.465Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🪛 Pylint (3.3.7)
holidays/countries/nauru.py
[convention] 28-28: Line too long (163/100)
(C0301)
[convention] 29-29: Line too long (108/100)
(C0301)
[convention] 33-33: Line too long (176/100)
(C0301)
[convention] 34-34: Line too long (176/100)
(C0301)
[convention] 35-35: Line too long (172/100)
(C0301)
[convention] 36-36: Line too long (176/100)
(C0301)
[convention] 37-37: Line too long (196/100)
(C0301)
[convention] 38-38: Line too long (177/100)
(C0301)
[convention] 39-39: Line too long (177/100)
(C0301)
[convention] 40-40: Line too long (185/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 124-124: Missing class docstring
(C0115)
[convention] 128-128: Missing class docstring
(C0115)
🔇 Additional comments (1)
holidays/countries/nauru.py (1)
111-120: Sequence of Christmas holidays may be flippedApplying
SAT_SUN_TO_NEXT_MON_TUEto both 25 Dec and 26 Dec causes 25 Dec (Sunday) ⇒ Tuesday and leaves 26 Dec (Monday) unchanged, giving Monday = 26 Dec, Tuesday = Christmas.
Most jurisdictions (and precedent within this library) expect Monday = Christmas, Tuesday = Day-after.Please verify Nauru’s practice and, if needed, switch to:
- rule=SAT_SUN_TO_NEXT_MON_TUE, + rule=SAT_SUN_TO_NEXT_MON,for Christmas Day only, keeping
SAT_SUN_TO_NEXT_MON_TUEfor the following day.
|
There was a problem hiding this comment.
This LGTM, @KJhellico @PPsyrius could you verify and approve?
|
Should I update branch now? |
I believe it's ready for merging. |
Proposed change
Add Nauru holidays.
Resolves #1233
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green