-
-
Notifications
You must be signed in to change notification settings - Fork 541
Add Nepal holidays #2386
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
Add Nepal holidays #2386
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThe changes introduce Nepal as a new supported country within the holiday management system. This includes updating the README to reflect its addition, creating a new module for Nepal's holiday functionality, updating the countries registry, and adding corresponding tests. The modifications ensure comprehensive holiday support for Nepal while maintaining integration with existing structures. Changes
Assessment against linked issues
Suggested reviewers
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 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (1)holidays/countries/__init__.py (1)
🔇 Additional comments (3)
✨ 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
|
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md
(2 hunks)holidays/countries/__init__.py
(1 hunks)holidays/countries/nepal.py
(1 hunks)holidays/registry.py
(1 hunks)tests/countries/test_nepal.py
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
tests/countries/test_nepal.py (1)
holidays/countries/nepal.py (3)
Nepal
(16-25)NP
(28-29)NPL
(32-33)
holidays/countries/__init__.py (1)
holidays/countries/nepal.py (3)
Nepal
(16-25)NP
(28-29)NPL
(32-33)
🔇 Additional comments (8)
holidays/registry.py (1)
126-126
: LGTM! Correctly added Nepal to the COUNTRIES registry.The Nepal entry follows the proper format with name and ISO codes (alpha-2 "NP" and alpha-3 "NPL"), and is correctly positioned alphabetically in the dictionary.
holidays/countries/__init__.py (1)
118-118
: LGTM! Nepal import statement correctly added.The import statement for Nepal and its code aliases follows the established pattern and is properly positioned alphabetically among other country imports.
tests/countries/test_nepal.py (1)
19-26
: LGTM! Test class correctly implements the required tests.The test class follows the project's testing pattern by:
- Inheriting from CommonCountryTests and TestCase
- Setting up the class with the Nepal holiday class
- Verifying that the country aliases (NP, NPL) correctly map to the Nepal class
This provides the necessary test coverage for the basic functionality.
README.md (2)
102-102
: LGTM! Correctly updated country count.The number of supported country codes has been properly updated from 160 to 161 to reflect the addition of Nepal.
865-870
: LGTM! Nepal entry correctly added to country table.The Nepal entry follows the established table format with the country name, ISO code (NP), supported language (en_US as default), and is correctly positioned alphabetically in the table.
holidays/countries/nepal.py (3)
13-14
: Import structure looks good.The import of HolidayBase is appropriate for creating a country-specific holiday class.
28-30
: Alias class implementation is correct.The NP class properly inherits from Nepal, following the project's convention of providing an alias using the ISO 3166-1 alpha-2 code.
32-34
: Alias class implementation is correct.The NPL class properly inherits from Nepal, following the project's convention of providing an alias using the ISO 3166-1 alpha-3 code.
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 (3)
holidays/countries/nepal.py
(1 hunks)holidays/groups/hindu.py
(7 hunks)tests/countries/test_nepal.py
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
tests/countries/test_nepal.py (1)
holidays/countries/nepal.py (3)
Nepal
(18-149)NP
(152-153)NPL
(156-157)
holidays/countries/nepal.py (1)
holidays/groups/hindu.py (7)
HinduCalendarHolidays
(21-396)_add_bhai_dooj
(62-72)_add_buddha_purnima
(74-85)_add_chhath_puja
(87-97)_add_gau_krida
(141-149)_add_govardhan_puja
(151-161)_add_tihar_holiday
(364-372)
holidays/groups/hindu.py (1)
holidays/calendars/hindu.py (3)
govardhan_puja_date
(1256-1257)maha_navami_date
(1274-1275)dussehra_date
(1250-1251)
🔇 Additional comments (15)
tests/countries/test_nepal.py (5)
1-12
: Looks solid for file headers and imports.
All new lines here appear properly structured, referencing correct modules.
19-23
: Validate super initialization call insetUpClass
.
Ensure thatsuper().setUpClass(Nepal)
indeed invokes any required parent initialization logic for this testing framework.Please confirm this invocation is behaving as expected in your test runner by checking test logs or coverage results.
24-26
: Straightforward test for country aliases.
The alias checks look correct and well-structured.
27-58
: Comprehensive holiday coverage for 2025.
All tested holidays match the expected date mappings. This is a clear, precise test.
60-231
: Extensive parametric holiday testing.
Covering multiple years for each holiday name is an excellent approach to ensure correctness.holidays/countries/nepal.py (3)
136-137
: Check for potential duplication in Mha Puja logic.
Invoking_add_govardhan_puja("Mha Puja")
shares code with Govardhan Puja. Confirm they share the exact same date in your references.Would you like a script to verify no date discrepancies exist in official references?
151-158
: Minimal stubs for NP and NPL
These subclasses correctly inherit from Nepal with no custom logic. They appear fine.
36-80
:✅ Verification successful
Verify that
_populate_public_holidays
is triggered.
It’s excellent to isolate holiday definitions in a dedicated method. However, confirm that the wider library calls_populate_public_holidays()
so these holidays actually appear in final results.
🏁 Script executed:
#!/bin/bash # Searching for `_populate_public_holidays` usage in the codebase. rg -A 5 '_populate_public_holidays'Length of output: 66509
Action: Confirm that Nepal’s
_populate_public_holidays
is correctly integrated.I've reviewed the codebase and found that the method defined in
holidays/countries/nepal.py
follows the same pattern as in other country modules. In multiple files, similar_populate_public_holidays
implementations are either directly or indirectly invoked—often through parent class methods (e.g., viasuper()._populate_public_holidays()
) or through the library’s routine population process. This confirms that Nepal’s holidays (including Prithvi Jayanti, Martyr’s Day, National Democracy Day, Republic Day, Constitution Day, Labor Day, and Nepali New Year) are set up to appear in the final results.Feel free to double-check with your local test suite if any country-specific outlets need further scrutiny.
holidays/groups/hindu.py (7)
31-42
: Parameterdays_delta
for_add_hindu_calendar_holiday
is a good addition.
This nicely enables holiday adjustments after Diwali or Dussehra. No immediate concerns.
62-72
: Bhai Tika addition is properly referencing Govardhan Puja date.
Implementation looks aligned with the festival’s chronology.
141-149
: Gau Krida logic is consistent.
The one-day offset (days_delta=-1
) aligns with the festival’s timing relative to Govardhan Puja.
155-155
: Slight docstring update for Govardhan Puja.
The clarification “celebrated after Diwali” is appropriate.
305-315
: Papankusha Ekadashi addition.
Implementation aligns with the standard calendar offset from Dussehra.
316-325
: Papankusha Duwadashi logic.
Adding one more day on top of Ekadashi is correct. This looks good.
364-372
: Tihar Holiday for Nepal.
Fifth day offset from Diwali (two days after Govardhan Puja) is consistent. Nicely done.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com>
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 (1)
holidays/groups/hindu.py
(7 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
holidays/groups/hindu.py (1)
holidays/calendars/hindu.py (3)
govardhan_puja_date
(1256-1257)maha_navami_date
(1274-1275)dussehra_date
(1250-1251)
🔇 Additional comments (6)
holidays/groups/hindu.py (6)
31-42
: Well-designed parameter enhancement.The addition of the
days_delta
parameter provides flexibility for defining holidays related to a specific date but observed on different days. This is a valuable enhancement for supporting the Nepal holidays.
62-72
: Implementation looks good.The Bhai Dooj holiday implementation correctly uses the Govardhan Puja date with an appropriate offset. The docstring is clear and informative.
141-149
: Implementation is correct.The Gau Krida holiday implementation effectively uses the Govardhan Puja date with a -1 day offset.
155-155
: Minor docstring improvement.The change from "celebrated the day after Diwali" to "celebrated after Diwali" makes the description more general and appropriate.
223-233
: Correct docstring and implementation.The Maha Saptami implementation correctly identifies it as the seventh day of Navratri and uses the appropriate date offset.
234-244
: Correct docstring and implementation.The Maha Ashtami implementation correctly identifies it as the eighth day of Navratri and uses the appropriate date offset.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com>
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: 2
♻️ Duplicate comments (1)
holidays/groups/hindu.py (1)
303-312
: 🧹 Nitpick (assertive)Add reference link to Papankusha Ekadashi docstring.
Similar to the suggestion in past review comments, add a reference link for completeness.
""" Add Papankusha Ekadashi. Papankusha Ekadashi is a Hindu festival which occurs on eleventh day on month of Ashwin (September-October). + https://en.wikipedia.org/wiki/Ekadashi """
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/groups/hindu.py
(7 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
holidays/groups/hindu.py (1)
holidays/calendars/hindu.py (3)
govardhan_puja_date
(1256-1257)maha_navami_date
(1274-1275)dussehra_date
(1250-1251)
🪛 Ruff (0.8.2)
holidays/groups/hindu.py
367-367: Trailing whitespace
Remove trailing whitespace
(W291)
368-368: Line too long (101 > 99)
(E501)
368-368: Trailing whitespace
Remove trailing whitespace
(W291)
🔇 Additional comments (6)
holidays/groups/hindu.py (6)
30-42
: Addition of days_delta parameter provides greater flexibility.The inclusion of a
days_delta
parameter in the_add_hindu_calendar_holiday
method is a solid enhancement. This allows for defining holidays that are offset from reference dates, making the implementation more versatile.
62-72
: Good implementation of Bhai Dooj.The implementation correctly derives Bhai Dooj by adding one day to Govardhan Puja. The docstring provides clear cultural context for the holiday.
155-155
: Clearer wording in Govardhan Puja docstring.The change from "celebrated the day after Diwali" to "celebrated after Diwali" is more accurate as it doesn't restrict to a specific day count.
223-233
: Maha Saptami implementation is correct.The implementation properly calculates Maha Saptami as two days before Maha Navami with an accurate docstring.
234-244
: Maha Ashtami implementation is correct.The implementation properly calculates Maha Ashtami as one day before Maha Navami with an accurate docstring.
314-323
: Papankusha Duwadashi implementation looks good.The implementation correctly derives the date and includes a reference link as suggested in past comments.
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: 1
♻️ Duplicate comments (1)
holidays/groups/hindu.py (1)
141-149
: 🧹 Nitpick (assertive)Consider adding a reference link for Gau Krida.
The method is implemented correctly, but the docstring would benefit from a reference link similar to other holiday methods in this class.
""" Add Gau Krida. Gau Krida, is celebrated the day after Diwali to honor cows. + https://en.wikipedia.org/wiki/Gau_Puja """
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/groups/hindu.py
(7 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
holidays/groups/hindu.py (1)
holidays/calendars/hindu.py (3)
govardhan_puja_date
(1256-1257)maha_navami_date
(1274-1275)dussehra_date
(1250-1251)
🔇 Additional comments (9)
holidays/groups/hindu.py (9)
31-31
: Update to method signature adds flexibility.The addition of the
days_delta
parameter with a default value of 0 is a good enhancement that allows for more precise holiday date calculations by offsetting dates when needed.
41-41
: Good implementation of the days_delta parameter.The parameter is correctly passed to the underlying
_add_eastern_calendar_holiday
method call, maintaining API consistency.
62-72
: Well-documented addition of Bhai Dooj holiday.The implementation correctly uses the Govardhan Puja date with a +1 day offset. The docstring provides good context about the festival and its relationship to Diwali.
155-155
: Good clarification in docstring.Changing "celebrated the day after Diwali" to "celebrated after Diwali" is more accurate and aligns better with the actual observance of the festival.
223-234
: Maha Saptami implementation looks good.The implementation correctly calculates the date by offsetting from Maha Navami by -2 days. The docstring provides appropriate context.
235-246
: Maha Ashtami implementation looks good.The implementation correctly calculates the date by offsetting from Maha Navami by -1 day. The docstring provides appropriate context.
305-316
: Good implementation of Papankusha Ekadashi.The method correctly uses the Dussehra date with a +1 day offset. Good job including the Wikipedia reference.
317-327
: Good implementation of Papankusha Duwadashi.The method correctly uses the Dussehra date with a +2 day offset. Good job including the Wikipedia reference.
366-379
: Good implementation of Tihar Holiday.The method correctly calculates the date by offsetting from Govardhan Puja by +2 days. This effectively adds support for Nepal holidays as mentioned in the PR objectives.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2386 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 217 218 +1
Lines 13783 13910 +127
Branches 1948 1954 +6
==========================================
+ Hits 13783 13910 +127 ☔ 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.
@ankushhKapoor, you have chosen not the easiest country! 👍
Just a few technical suggestions for now, I want to go a little deeper into the calendar and holidays of Nepal. What sources did you rely on?
Thank you! 😊
I referred to Wikipedia , Time and Date and HamroPatro calendar as sources since Nepal doesn’t have an official website listing all holidays. |
I found this one from 2024 with both original Nepalese language document and English translation: https://narayanilawfirm.org.np/list-of-public-holidays-in-nepal-2079/ |
This is nice. I will check and update holidays in the project if needed. |
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com>
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
♻️ Duplicate comments (2)
holidays/countries/nepal.py (2)
274-280
: 🧹 Nitpick (assertive)Typo: “Gobardhan” → “Govardhan” (v ≠ b)
The helper is_add_govardhan_puja
; aligning the holiday label avoids duplicate spellings in downstream reports.- # Gobardhan Pooja. - self._add_govardhan_puja("Gobardhan Pooja") + # Govardhan Puja. + self._add_govardhan_puja("Govardhan Puja")
160-168
: 🧹 Nitpick (assertive)Boolean expression hard to read – split for clarity
if (2019 <= self._year <= 2020) == is_workday:
forces a mental double-take.
A small local variable keeps intent obvious:- if (2019 <= self._year <= 2020) == is_workday: + in_cut_years = 2019 <= self._year <= 2020 + if (is_workday and in_cut_years) or (not is_workday and not in_cut_years):
📜 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 (1)
holidays/countries/nepal.py
(1 hunks)
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: 2
♻️ Duplicate comments (2)
holidays/countries/nepal.py (2)
270-274
: 🧹 Nitpick (assertive)Harmonise spelling: “Govardhan Puja”
The library’s helper is named
_add_govardhan_puja
; every public source linked in the header uses “Govardhan”.
Keeping a single spelling avoids duplicate holiday labels that differ only by a typo.- # Gobardhan Pooja. - self._add_govardhan_puja("Gobardhan Pooja") + # Govardhan Puja. + self._add_govardhan_puja("Govardhan Puja")(If “Gobardhan” is intentional, maybe leave a short comment explaining the regional form.)
154-167
: 🛠️ Refactor suggestionRefactor the “double-negative” condition for clarity
The comparison
(2019 <= self._year <= 2020) == is_workday
forces a mental double-take every time somebody reads it.
A tiny helper variable makes the intent obvious, costs nothing in performance, and follows the style already used elsewhere in the code-base.- if (2019 <= self._year <= 2020) == is_workday: + in_cut_years = 2019 <= self._year <= 2020 + if (is_workday and in_cut_years) or (not is_workday and not in_cut_years):Same logic, easier maintenance.
📜 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 (1)
holidays/countries/nepal.py
(1 hunks)
🔇 Additional comments (1)
holidays/countries/nepal.py (1)
234-241
: Check chronological ordering within Hindu block
Maha Shivaratri
(Feb 29 / Mar 8) generally precedesGyalpo Lhosar
(Feb — dates vary).
If strict Gregorian chronology is the rule (per earlier reviewer guidance), consider swapping these two calls.
I agree with the current names. 👍 |
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! (l10n can be made as separate PR)
Thanks! Sure, I’ll make l10n a separate PR. |
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: 5
📜 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 (1)
holidays/countries/nepal.py
(1 hunks)
🔇 Additional comments (3)
holidays/countries/nepal.py (3)
169-171
: Three spellings for Janai Purnima – pick one
Janai Poornima
(here),Janai Punima
(constant name), and the common EnglishJanai Purnima
all appear.
Inconsistent labels create near-duplicate entries in the final calendar.Action: settle on the preferred spelling (project usually follows Wikipedia → “Janai Purnima”) and update all occurrences.
265-269
: Verify “Gobardhan Pooja” vs “Govardhan Puja”
Local transliteration may differ, but elsewhere in Hindu modules the V-spelling is used. Double-check which form Nepali government sources prefer; otherwise consumers will see two holidays that differ only by a letter.Would you like a quick grep across the repo to confirm current usage patterns?
409-441
: Special-holiday table drives behaviour – guard against stray tuple nesting
Most years map to a single 3-tuple; 2023 maps to a tuple-of-tuples.
Looks correct, but a unit-test assertingisinstance(value[0], int)
for single-holiday years would catch accidental double-nesting (a past bug already fixed for 2011/2024).
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.
@ankushhKapoor thanks for this tremendous and long-awaited contribution!
I have a couple of minor comments regarding the code formatting but this is 💯 mergable as both @KJhellico and @PPsyrius LGTMed it.
|
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.
275 conversation, 70 commits 🤯
A lot of great work, thank you @ankushhKapoor @KJhellico @PPsyrius 🎉
Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <ark@cho.red>
Signed-off-by: Ankush_Kapoor <work.ankushkapoor1626@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <ark@cho.red>
Proposed change
Added Nepal holidays.
Resolves #1234
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green