Update India holidays: add Bonalu holiday in Telangana (TS)#3426
Update India holidays: add Bonalu holiday in Telangana (TS)#3426arkid15r merged 12 commits intovacanza:devfrom
Conversation
Vikash-Kumar-23
requested review from
KJhellico,
PPsyrius and
arkid15r
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Bonalu: new Hindu calendar constant and date accessor, a Hindu group helper to register Bonalu, registration for Telangana in the India provider, tests asserting Bonalu dates, and localized translations for multiple locales. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds Bonalu as a Telangana (TS) subdivision public holiday for a confirmed 2026 date, while preserving the TG → TS alias behavior in the India holiday provider.
Changes:
- Added a year-specific Bonalu date mapping (2026-08-10) to the India provider and populated it for Telangana only when available for the current year.
- Added tests asserting Bonalu appears for both
subdiv="TS"and thesubdiv="TG"alias on 2026-08-10.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
holidays/countries/india.py |
Adds a Bonalu date mapping and includes it in Telangana subdivision public holidays only for mapped years. |
tests/countries/test_india.py |
Adds regression tests for Bonalu presence for TS and TG on the confirmed 2026 date. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/india.py`:
- Around line 56-58: Move the BONALU_DATES mapping into the Telangana-specific
population method so it lives next to the logic that consumes it; find the
BONALU_DATES symbol and relocate its definition inside the Telangana populate
function (the method that adds Telangana holidays) and use the local mapping
there, removing the top-level constant to keep year-to-date dictionaries scoped
to their populate methods.
In `@tests/countries/test_india.py`:
- Around line 986-990: Add a negative unit test that ensures Bonalu is not
present for a year without mapping: create a new test (e.g.,
test_bonalu_not_mapped_year) that calls self.assertNoHolidayName("Bonalu",
India(subdiv="TS"), "2025-08-10") and also checks the TG alias with
self.assertNoHolidayName("Bonalu", India(subdiv="TG"), "2025-08-10"); place it
alongside the existing test_bonalu_ts_2026 and test_bonalu_tg_alias_2026 so it
verifies the “only when mapping exists” constraint.
🪄 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: a576e282-a242-4958-aeb1-ce13baf9c815
📒 Files selected for processing (2)
holidays/countries/india.pytests/countries/test_india.py
|
If we're going to add holiday support, we need to do it fully. Doing it this way, just for one year, doesn't make sense. |
|
I could only find reliable sources for Bonalu public holiday dates up to 2028:
No confirmed dates available yet for 2029 and 2030 so should i added only up to 2028 for now? |
|
This is a Lunisolar calendar holiday, similar to many others in India.
|
|
@KJhellico I went through multiple Hindu/panchang calendar websites (Astrosage, Prokerala, mpanchang, Astroyogi, etc.), but Bonalu dates vary quite a bit across sources, and many websites don't even mention it, and the public holiday date declared by the Telangana government doesn't always match the main temple celebration day. |
|
Hi @KJhellico |
That would be fine. But this holiday should be implemented via |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/countries/test_india.py (1)
1190-1194:⚠️ Potential issue | 🟡 MinorAdd the missing unmapped-year negative check for Bonalu.
These tests confirm presence in 2026, but they still don’t validate the “only when mapping exists” constraint.
✅ Suggested test addition
def test_bonalu_tg_alias_2026(self): self.assertHolidayName("Bonalu", India(subdiv="TG"), "2026-08-10") + + def test_bonalu_not_added_when_unmapped(self): + self.assertNoHolidayName("Bonalu", India(subdiv="TS"), "2025-08-10") + self.assertNoHolidayName("Bonalu", India(subdiv="TG"), "2025-08-10")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/countries/test_india.py` around lines 1190 - 1194, Add a negative test to assert Bonalu is not present in an unmapped year (e.g., 2025) for the same subdivisions; create test methods like test_bonalu_ts_unmapped_2025 and test_bonalu_tg_unmapped_2025 that instantiate India(subdiv="TS") and India(subdiv="TG") and use the test suite’s negative holiday assertion (e.g., assertNoHoliday or equivalent) for the date "2025-08-10" to ensure the holiday only appears when mapping exists.holidays/countries/india.py (1)
60-62:⚠️ Potential issue | 🟠 MajorSingle-year Bonalu mapping leaves Telangana coverage incomplete.
This adds Bonalu only for 2026 and silently omits it for every other year, which is a functional gap for TS/TG. Please model Bonalu through the Hindu/lunisolar path and keep explicit date overrides only for years with confirmed government notifications.
Based on learnings: explicit date dictionaries are typically kept for confirmed years, while helper-based calendar methods provide broader year coverage.
Also applies to: 502-505
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holidays/countries/india.py` around lines 60 - 62, The BONALU_DATES dict currently hardcodes only 2026 which omits Bonalu for other years; replace the single-year mapping by computing Bonalu via the existing Hindu/lunisolar helper method used elsewhere in india.py (use the same helper function/class that computes lunisolar festival dates) and retain an overrides dict (BONALU_DATES) only for government-notified exceptional years; update references to BONALU_DATES and the Bonalu computation logic (where Bonalu is used) so the code falls back to the lunisolar-calculated date unless an explicit override exists—apply the same change pattern to the other explicit-year mappings mentioned in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@holidays/countries/india.py`:
- Around line 60-62: The BONALU_DATES dict currently hardcodes only 2026 which
omits Bonalu for other years; replace the single-year mapping by computing
Bonalu via the existing Hindu/lunisolar helper method used elsewhere in india.py
(use the same helper function/class that computes lunisolar festival dates) and
retain an overrides dict (BONALU_DATES) only for government-notified exceptional
years; update references to BONALU_DATES and the Bonalu computation logic (where
Bonalu is used) so the code falls back to the lunisolar-calculated date unless
an explicit override exists—apply the same change pattern to the other
explicit-year mappings mentioned in the file.
In `@tests/countries/test_india.py`:
- Around line 1190-1194: Add a negative test to assert Bonalu is not present in
an unmapped year (e.g., 2025) for the same subdivisions; create test methods
like test_bonalu_ts_unmapped_2025 and test_bonalu_tg_unmapped_2025 that
instantiate India(subdiv="TS") and India(subdiv="TG") and use the test suite’s
negative holiday assertion (e.g., assertNoHoliday or equivalent) for the date
"2025-08-10" to ensure the holiday only appears when mapping exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e38ae390-1206-487b-b1e7-962b6675cdbb
📒 Files selected for processing (2)
holidays/countries/india.pytests/countries/test_india.py
KJhellico
left a comment
KJhellico
left a comment
There was a problem hiding this comment.
When adding new holidays, you need to update .po files (run make l10n) and add translations.
KJhellico
changed the title
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/countries/test_india.py (1)
291-305:⚠️ Potential issue | 🟡 MinorTest coverage for mapped years looks good. Add a negative test for unmapped years.
The positive tests correctly cover all dates in
BONALU_DATES(2018-2027). However, the "only when mapping exists" constraint should also be verified with a negative case.✅ Suggested negative test addition
for subdiv in {"TS"}: self.assertHolidayName(name, India(subdiv=subdiv), dt) + + # Verify Bonalu is not present for years outside the mapping + self.assertNoHolidayName("Bonalu", India(subdiv="TS", years=2017)) + self.assertNoHolidayName("Bonalu", India(subdiv="TS", years=2028))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/countries/test_india.py` around lines 291 - 305, Add a negative test to verify Bonalu is not present for years outside the mapped range: after the positive loop that uses self.assertHolidayName(name, India(subdiv=subdiv), dt), add assertions for at least one year before and one year after the mapping (e.g., 2017 and 2028) that the holiday is not present for the Telangana subdivision by calling the complementary check (use self.assertNoHoliday with India(subdiv="TS") or the project’s equivalent “no holiday” assertion) to confirm unmapped years do not return the Bonalu holiday.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/countries/test_india.py`:
- Around line 291-305: Add a negative test to verify Bonalu is not present for
years outside the mapped range: after the positive loop that uses
self.assertHolidayName(name, India(subdiv=subdiv), dt), add assertions for at
least one year before and one year after the mapping (e.g., 2017 and 2028) that
the holiday is not present for the Telangana subdivision by calling the
complementary check (use self.assertNoHoliday with India(subdiv="TS") or the
project’s equivalent “no holiday” assertion) to confirm unmapped years do not
return the Bonalu holiday.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f65d4907-95d6-419a-ae68-8472433b85c1
📒 Files selected for processing (2)
holidays/groups/hindu.pytests/countries/test_india.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/en_IN/LC_MESSAGES/IN.po`:
- Around line 312-314: Update the default-locale PO entry for the Bonalu holiday
so its msgstr is empty per project convention: locate the entry with msgid
"Bonalu" and replace the current msgstr "Bonalu" with an empty msgstr "" so the
system will fall back to the msgid; ensure formatting/comment lines remain
unchanged and follow other entries in this file.
🪄 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: c9e2bbf4-1955-4085-b342-5751553a3873
📒 Files selected for processing (10)
holidays/locale/bn/LC_MESSAGES/IN.poholidays/locale/en_IN/LC_MESSAGES/IN.poholidays/locale/en_US/LC_MESSAGES/IN.poholidays/locale/gu/LC_MESSAGES/IN.poholidays/locale/hi/LC_MESSAGES/IN.poholidays/locale/kn/LC_MESSAGES/IN.poholidays/locale/mr/LC_MESSAGES/IN.poholidays/locale/pa/LC_MESSAGES/IN.poholidays/locale/ta/LC_MESSAGES/IN.poholidays/locale/te/LC_MESSAGES/IN.po
Signed-off-by: Vikash Kumar <[email protected]>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3426 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 313 313
Lines 18659 18666 +7
Branches 2383 2383
=========================================
+ Hits 18659 18666 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@Vikash-Kumar-23, now you need to merge latest changes from |
|
@KJhellico |
|
Proposed change
This PR adds the missing Bonalu public holiday for Telangana in the India provider.
Fixes #3232
Changes included:
Added a year-specific Bonalu mapping for confirmed dates:
2026-08-10
Updated Telangana subdivision holiday population to add Bonalu only when a mapping exists for the current year.
Kept TG alias behavior unchanged (TG continues to resolve to TS).
Added tests to verify:
India(subdiv="TS", years=2026) includes Bonalu on 2026-08-10
India(subdiv="TG", years=2026) includes Bonalu on 2026-08-10
Modified files:
holidays/countries/india.py
tests/countries/test_india.py
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.