Update India holidays: subdiv holidays improvement (North region) - #3666
Merged
Conversation
Contributor
|
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:
Summary by CodeRabbit
WalkthroughAdds Hindu lunisolar observances, expands India subdivision holiday rules and categories, updates localized India catalogs, and broadens subdivision and localization tests. ChangesIndia Hindu holiday support
Localized India catalogs
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is part of the ongoing India subdivision-holidays accuracy work. It expands India’s subdivision holiday definitions (notably Himachal Pradesh and Punjab) and adds Hindu-calendar support needed for new subdivision holidays, with corresponding test and localization updates.
Changes:
- Added Hindu lunisolar calendar support for Kabir Jayanti and Parshuram Jayanti, and exposed them via
HinduCalendarHolidays. - Updated India subdivision holiday population (HP/PB additions, PB special-case dates) and reorganized/expanded India subdivision holiday tests.
- Updated IN locale
.pofiles with new holiday keys (and revision timestamps).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 27 comments.
Show a summary per file
| File | Description |
|---|---|
holidays/calendars/hindu.py |
Adds new lunisolar date tables and accessors for Kabir Jayanti and Parshuram Jayanti. |
holidays/groups/hindu.py |
Adds helpers to register Kabir Jayanti and Parshuram Jayanti holidays via the Hindu calendar. |
holidays/countries/india.py |
Adds/adjusts HP + PB subdivision holidays and PB special public holiday dates. |
tests/countries/test_india.py |
Updates/expands subdivision holiday assertions and reorganizes subdiv test sections. |
holidays/locale/bn/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/en_IN/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/en_US/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/gu/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/hi/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/kn/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/ml/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/mr/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/pa/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/ta/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
holidays/locale/te/LC_MESSAGES/IN.po |
Adds new holiday msgids for India subdivision updates. |
💡 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@holidays/calendars/hindu.py`:
- Around line 1977-1979: The kabir_jayanti_date accessor is out of alphabetical
order relative to the other holiday accessors in hindu.py. Move
kabir_jayanti_date so it appears before karwa_chauth_date, keeping the accessor
method order aligned with the KABIR_JAYANTI_DATES and KARWA_CHAUTH_DATES
dictionary order and matching the existing pattern used by the class.
In `@holidays/countries/india.py`:
- Around line 668-669: The nearby comment in the India holidays logic does not
match the translated label used in self._add_womens_day(tr("International
Women's Day")). Update the comment so it exactly reflects the same wording as
the tr() string, keeping the symbol _add_womens_day as the reference point.
- Around line 641-644: The translator-facing comment for the holiday entry in
India’s holiday list does not exactly match the string passed to tr() because it
omits the hyphen in “Shaheed-e-Azam”. Update the comment near the
_add_holiday_mar_23 call in the india holiday definitions so it matches the
translated string exactly, keeping the wording aligned with the tr("Shaheedi
Diwas of Shaheed-e-Azam Bhagat Singh, Sukhdev and Rajguru Ji") text.
- Around line 549-555: The `_populate_subdiv_hp_women_optional_holidays` method
currently repeats country-wide optional holidays, so remove the duplicate
`Raksha Bandhan` and `Bhai Duj` additions and keep only the distinct `Karwa
Chouth` entry. Update the HP-specific holiday population logic in this method so
it only adds holidays not already provided by the shared optional holiday set.
In `@holidays/locale/bn/LC_MESSAGES/IN.po`:
- Around line 407-408: The Bengali locale entry for this holiday is missing the
translator note already present in holidays/countries/india.py. Regenerate the
locale files by rerunning make l10n so the note is synced into
holidays/locale/bn/LC_MESSAGES/IN.po, rather than editing the PO file manually.
In `@holidays/locale/en_US/LC_MESSAGES/IN.po`:
- Around line 409-462: This block in IN.po contains multiple untranslated
holiday entries with empty msgstr values, which is inconsistent with the rest of
the file. Populate the translations for each affected msgid in this section,
keeping the existing msgid text intact and ensuring every entry in this block
has a non-empty localized msgstr. Use the nearby holiday entries in the same
locale file as the reference pattern when updating these strings.
- Around line 324-354: The new en_US holiday entries are missing their
translated msgstr values, which breaks the file’s existing convention of
providing non-empty locale labels. Update the untranslated entries in the IN.po
holiday section so msgstr is populated consistently for Statehood Day, Guru
Ravidas's Jayanti, Bhagvan Shree Parshuram Jayanti, Sant Guru Kabir Jayanti
(Prakat Diwas), Maharishi Valmiki's Birthday, and Karwa Chouth, matching the
style used by existing entries like Himachal Day and Maharana Pratap Jayanti.
In `@holidays/locale/hi/LC_MESSAGES/IN.po`:
- Around line 407-409: The new Punjab holiday entry is missing its extracted
translator comment because the source comment in india.py does not exactly match
the tr() string. Update the preceding comment for the Shaheedi Diwas entry in
india.py so it matches the msgid text exactly (including the “Shaheed-e-Azam”
spelling), then regenerate the locale files with make l10n so the #. comment is
re-extracted into the PO 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: 56dfa52f-e895-4593-a7be-ba09f0ecde35
📒 Files selected for processing (15)
holidays/calendars/hindu.pyholidays/countries/india.pyholidays/groups/hindu.pyholidays/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/ml/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.potests/countries/test_india.py
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 15 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
holidays/countries/india.py (2)
663-665: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLohri duplicated for Punjab in overlapping years.
_populate_subdiv_pb_optional_holidaysnow addsself._add_lohri(tr("Lohri"))unconditionally underOPTIONAL, but_populate_optional_holidaysalready adds the identical name for 2020-2022 and 2024 (Line 316-318). Both fire together for PB, producing "Lohri; Lohri" in those years. The AI summary notes Lohri was moved from public to optional here, which is what introduces the collision with the existing country-wide conditional Lohri.🐛 Proposed fix
def _populate_subdiv_pb_optional_holidays(self): - # Lohri. - self._add_lohri(tr("Lohri")) + if not (2020 <= self._year <= 2022 or self._year == 2024): + # Lohri. + self._add_lohri(tr("Lohri")) # Birthday of Satguru Ram Singh Ji. self._add_basant_panchami(tr("Birthday of Satguru Ram Singh Ji"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/countries/india.py` around lines 663 - 665, The Punjab optional holiday population is duplicating Lohri because _populate_subdiv_pb_optional_holidays now always calls _add_lohri(tr("Lohri")) while _populate_optional_holidays already adds the same holiday for specific years. Update the logic in _populate_subdiv_pb_optional_holidays and/or _populate_optional_holidays so Lohri is only added once for PB in overlapping years, preserving the intended year-based behavior without creating duplicate entries.
544-545: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDussehra duplicated for Himachal Pradesh.
_populate_subdiv_hp_public_holidayscallsself._add_dussehra(tr("Dussehra"))(Line 545), but_populate_public_holidaysalready adds the identical name under the samePUBLICcategory for every subdiv (Line 269-272). Since names merge on collision, HP would show "Dussehra; Dussehra" on that date for the defaultPUBLICcategory.🐛 Proposed fix
def _populate_subdiv_hp_public_holidays(self): # Statehood Day. self._add_holiday_jan_25(tr("Statehood Day")) # Guru Ravidas's Jayanti. self._add_guru_ravidas_jayanti(tr("Guru Ravidas's Jayanti")) # Himachal Day. self._add_holiday_apr_15(tr("Himachal Day")) # Bhagvan Shree Parshuram Jayanti self._add_parshuram_jayanti(tr("Bhagvan Shree Parshuram Jayanti")) # Maharana Pratap Jayanti. self._add_maharana_pratap_jayanti(tr("Maharana Pratap Jayanti")) # Sant Guru Kabir Jayanti (Prakat Diwas). self._add_kabir_jayanti(tr("Sant Guru Kabir Jayanti (Prakat Diwas)")) - # Dussehra. - self._add_dussehra(tr("Dussehra")) # Maharishi Valmiki's Birthday. self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki's Birthday"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/countries/india.py` around lines 544 - 545, The Himachal Pradesh holiday population is adding Dussehra twice under the same PUBLIC category, causing duplicate merged names. Update _populate_subdiv_hp_public_holidays to avoid calling self._add_dussehra(tr("Dussehra")) when _populate_public_holidays already adds it for all subdivisions, or otherwise make the HP-specific path skip the duplicate entry. Use the existing _populate_public_holidays and _populate_subdiv_hp_public_holidays logic to keep the PUBLIC holiday list deduplicated.tests/countries/test_india.py (2)
433-445: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest won't catch the Lohri duplication.
This only checks that "Lohri" is present for PB, not that it's the sole entry for that date. It won't catch the "Lohri; Lohri" duplication described in the india.py comment for years 2020-2022/2024. Once the source fix lands, consider asserting exact holiday name equality for those years to guard against regressions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/countries/test_india.py` around lines 433 - 445, The Lohri test only verifies presence, so it won’t detect duplicate holiday names like “Lohri; Lohri” in the Punjab subdivision cases. Update test_lohri in test_india.py to assert the exact holiday name returned by the holiday helper for the relevant years, using the existing _assertHinduHolidayHelper and assertSubdivPbOptionalHolidayName entry points so the check fails if the name is duplicated again.
801-816: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider testing absence for years before 2012.
assertSubdivHpWomenOptionalHolidayNameonly checks the 2020-2025 dates; it doesn't confirm behavior for years before 2012 where the country-wide equivalent is guarded off. Once the guard question in india.py is resolved, add coverage for that boundary here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/countries/test_india.py` around lines 801 - 816, The Karwa Chouth test only verifies the optional holiday dates for 2020–2025 and misses the pre-2012 boundary where the country-wide holiday is disabled. Update test_karwa_chouth in the India country tests to also assert absence for years before 2012, using the existing _assertHinduHolidayHelper and assertSubdivHpWomenOptionalHolidayName coverage pattern so the boundary behavior in india.py is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@holidays/countries/india.py`:
- Around line 663-665: The Punjab optional holiday population is duplicating
Lohri because _populate_subdiv_pb_optional_holidays now always calls
_add_lohri(tr("Lohri")) while _populate_optional_holidays already adds the same
holiday for specific years. Update the logic in
_populate_subdiv_pb_optional_holidays and/or _populate_optional_holidays so
Lohri is only added once for PB in overlapping years, preserving the intended
year-based behavior without creating duplicate entries.
- Around line 544-545: The Himachal Pradesh holiday population is adding
Dussehra twice under the same PUBLIC category, causing duplicate merged names.
Update _populate_subdiv_hp_public_holidays to avoid calling
self._add_dussehra(tr("Dussehra")) when _populate_public_holidays already adds
it for all subdivisions, or otherwise make the HP-specific path skip the
duplicate entry. Use the existing _populate_public_holidays and
_populate_subdiv_hp_public_holidays logic to keep the PUBLIC holiday list
deduplicated.
In `@tests/countries/test_india.py`:
- Around line 433-445: The Lohri test only verifies presence, so it won’t detect
duplicate holiday names like “Lohri; Lohri” in the Punjab subdivision cases.
Update test_lohri in test_india.py to assert the exact holiday name returned by
the holiday helper for the relevant years, using the existing
_assertHinduHolidayHelper and assertSubdivPbOptionalHolidayName entry points so
the check fails if the name is duplicated again.
- Around line 801-816: The Karwa Chouth test only verifies the optional holiday
dates for 2020–2025 and misses the pre-2012 boundary where the country-wide
holiday is disabled. Update test_karwa_chouth in the India country tests to also
assert absence for years before 2012, using the existing
_assertHinduHolidayHelper and assertSubdivHpWomenOptionalHolidayName coverage
pattern so the boundary behavior in india.py is exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01d48d84-42f1-4c23-a9ef-37927f22411c
📒 Files selected for processing (3)
holidays/constants.pyholidays/countries/india.pytests/countries/test_india.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@holidays/calendars/hindu.py`:
- Line 32: The holiday lookup is using the suffixed constant name, which causes
`_get_holiday()` to append `_DATES` twice and fail to resolve the martyrdom
holiday. Rename the module constant `GURU_ARJUN_DEV_MARTYRDOM_DATES` to
`GURU_ARJUN_DEV_MARTYRDOM` and update the `_get_holiday()` call sites in
`hindu.py` to pass that base key so the internal suffixing works correctly for
JK/LA/PB.
In `@holidays/countries/india.py`:
- Around line 683-684: The PB Guru Arjun Dev Martyrdom Day assertion is using
the wrong holiday bucket in the India tests. Update
tests/countries/test_india.py::test_guru_arjun_dev_martyrdom_day to use the
public subdivision helper instead of assertSubdivPbOptionalHolidayName, or
alternatively move the registration in
India::_populate_subdiv_pb_public_holidays if the holiday is meant to be
optional. Keep the test and the holiday registration in india.py consistent with
the bucket where _add_guru_arjun_dev_martyrdom_day is actually added.
In `@tests/countries/test_india.py`:
- Around line 1216-1231: The Punjab holiday check in
test_guru_arjun_dev_martyrdom_day is using the optional-holiday assertion for
name_pb even though that label is populated by
_populate_subdiv_pb_public_holidays. Update the test to use the public Punjab
holiday assertion for name_pb, while keeping the JK/LA optional checks and the
no-holiday assertions aligned with the existing names.
- Around line 1030-1040: Split the combined JK/LA coverage in
test_chaitra_navratri so it matches the pattern used by test_vaisakhi: keep
_assertHinduHolidayHelper for the JK public subdiv holiday only, and add a
separate assertSubdivLaOptionalHolidayName assertion for LA. Also update the
holiday label from "1st Navratri" to "1st Navratra" so the test name matches the
expected holiday naming.
🪄 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: 7eabc8bb-7886-48a2-ad5e-53ae3f0ed7af
📒 Files selected for processing (5)
README.mdholidays/calendars/hindu.pyholidays/countries/india.pyholidays/groups/hindu.pytests/countries/test_india.py
Contributor
There was a problem hiding this comment.
4 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="holidays/locale/hi/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/hi/LC_MESSAGES/IN.po:326">
P2: Several newly added holiday entries in the Hindi locale file (`holidays/locale/hi/LC_MESSAGES/IN.po`) are left with empty `msgstr ""` values. Because GNU gettext falls back to the source `msgid` when a translation is empty, users with the `hi` locale will see English names for those holidays, producing a mixed Hindi/English experience. Since every pre-existing entry in this file is already fully translated, the blank entries appear to be an oversight rather than an intentional convention. Please add the corresponding Hindi translations for each new `msgid` so the locale remains complete and consistent.</violation>
</file>
<file name="holidays/locale/mr/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/mr/LC_MESSAGES/IN.po:328">
P2: The newly added Marathi `.po` entries for `Guru Ravidas's Jayanti`, `Karwa Chouth`, and `Maharishi Valmiki's Birthday` are untranslated aliases of already-translated holidays (`Guru Ravi Das's Jayanti`, `Karaka Chaturthi (Karwa Chouth)`, and `Maharishi Valmiki's Jayanti`). Since different Indian subdivisions emit these different msgids for the same underlying holiday, users will see English fallback in some states and Marathi translations in others, creating an inconsistent localization experience. Consider either reusing the existing msgids in the source Python code (avoiding alias proliferation that the project convention warns against) or copying the equivalent Marathi translations into the new entries.</violation>
</file>
<file name="holidays/locale/ml/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/ml/LC_MESSAGES/IN.po:328">
P2: The newly added holiday label `Guru Ravidas's Jayanti` is a variant spelling of the existing `Guru Ravi Das's Jayanti`, which already has a Malayalam translation in this file. Because both labels map to the same underlying holiday method in the source code (`_add_guru_ravidas_jayanti`), users in one subdivision will see Malayalam while users in another will see untranslated English. This fragments localization and creates maintenance drift—each variant must be translated separately even though it refers to the same observance. Prefer a single canonical English label in the source `.py` for each holiday so the generated `.po` can reuse one translated entry across all subdivisions.</violation>
</file>
<file name="holidays/locale/en_IN/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/en_IN/LC_MESSAGES/IN.po:444">
P2: Getexact `msgid` lookup means any typo or wording mismatch between a `.po` entry and the source `tr()` string creates a dead translation. Several newly added `msgid`s in this file do not match the literals used in `holidays/countries/india.py`:
- `International Women Day` should be `International Women's Day` (source: `india.py:669`).
- `Birthday of Sri Guru Nabha Dass ji` should be `Birthday of Sri Guru Nabha Dass Ji` (source: `india.py:646`).
- `Martyrdom Day of S. Kartar Singh Sarabha ji` should be `Martyrdom Day of S. Kartar Singh Sarabha Ji` (source: `india.py:658`).
- `Martyrdom Day of Sri Guru Teg Bahadur ji` should be `Martyrdom Day of Sri Guru Teg Bahadur Ji` (source: `india.py:661`).
Because the same incorrect `msgid`s are present in all India locale files, any future or existing translations for those strings will never be resolved at runtime. Please regenerate the `.po` files with `make l10n` or correct the `msgid`s to exactly match the source strings.</violation>
</file>
<file name="holidays/calendars/hindu.py">
<violation number="1" location="holidays/calendars/hindu.py:32">
P1: The module constant is named `GURU_ARJUN_DEV_MARTYRDOM_DATES` with value `"GURU_ARJUN_DEV_MARTYRDOM_DATES"`, but `_get_holiday()` appends `_DATES` internally to locate the class attribute. This means the lookup will search for `GURU_ARJUN_DEV_MARTYRDOM_DATES_DATES`, which doesn't exist, so this holiday will never resolve for JK/LA/PB.
Rename the module constant to `GURU_ARJUN_DEV_MARTYRDOM = "GURU_ARJUN_DEV_MARTYRDOM"` (keeping the class dates dict as `GURU_ARJUN_DEV_MARTYRDOM_DATES`) and pass `GURU_ARJUN_DEV_MARTYRDOM` to `_get_holiday()`.</violation>
</file>
<file name="tests/countries/test_india.py">
<violation number="1" location="tests/countries/test_india.py:1040">
P1: Two issues here:
1. The name `"1st Navratri"` doesn't match the registered holiday name `"1st Navratra"` in `india.py` (for both JK and LA).
2. `_assertHinduHolidayHelper(..., subdivs={"JK", "LA"})` tests public subdiv holidays, but LA registers this holiday under `_populate_subdiv_la_optional_holidays`. LA needs to be asserted separately with `assertSubdivLaOptionalHolidayName` (similar to how `test_vaisakhi` handles this split).</violation>
<violation number="2" location="tests/countries/test_india.py:1229">
P1: `name_pb` ("Martyrdom Day of Sri Guru Arjun Dev Ji") is registered under `_populate_subdiv_pb_public_holidays` in `india.py`, but the test asserts it with `assertSubdivPbOptionalHolidayName`. This tests the wrong category — it should use the public holiday assertion helper instead.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
holidays/countries/india.py (1)
579-581: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuru Arjun Dev Martyrdom Day name capitalization differs between JK and LA.
JK registers
"Martyr's Day of Guru Arjun Dev Ji"(capital "Day", Line 581) while LA registers"Martyr's day of Guru Arjun Dev Ji"(lowercase "day", Line 619).tests/countries/test_india.py::test_guru_arjun_dev_martyrdom_dayuses a singlenamevariable for bothassertSubdivJkOptionalHolidayNameandassertSubdivLaOptionalHolidayName, so one of these assertions will fail unless the strings match exactly.🐛 Proposed fix (align on capital "Day")
def _populate_subdiv_la_optional_holidays(self): # Shab-I-Miraj. self._add_isra_and_miraj_day(tr("Shab-I-Miraj")) # 1st Navratra. self._add_chaitra_navratri(tr("1st Navratra")) # Baisakhi / Vaisakhi. self._add_vaisakhi(tr("Baisakhi / Vaisakhi")) # Eid-e-Ghadeer. self._add_eid_al_ghadir_day(tr("Eid-e-Ghadeer")) - # Martyr's day of Guru Arjun Dev Ji. - self._add_guru_arjun_dev_martyrdom_day(tr("Martyr's day of Guru Arjun Dev Ji")) + # Martyr's Day of Guru Arjun Dev Ji. + self._add_guru_arjun_dev_martyrdom_day(tr("Martyr's Day of Guru Arjun Dev Ji"))Also applies to: 609-619
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/countries/india.py` around lines 579 - 581, The Guru Arjun Dev Martyrdom Day holiday names are inconsistent between JK and LA, causing the shared test expectation to fail. Update the relevant holiday registration in the India holiday helpers, especially _populate_subdiv_jk_optional_holidays and the matching LA subdivision method, so both use the same capitalization for the holiday name. Keep the string aligned with the shared assertion in tests/countries/test_india.py::test_guru_arjun_dev_martyrdom_day.tests/countries/test_india.py (1)
794-807: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing subdiv coverage for HP's "Maharishi Valmiki's Birthday".
HP registers a third, distinct name (
"Maharishi Valmiki's Birthday", india.py Line 552) for this holiday, separate from the country-wide"Maharishi Valmiki's Jayanti"and PB's"Birthday of Maharishi Valmiki Ji". Only the PB variant is asserted here; add an HP assertion so the new subdiv label is actually covered.✅ Suggested addition
def test_maharishi_valmiki_jayanti(self): name = "Maharishi Valmiki's Jayanti" name_pb = "Birthday of Maharishi Valmiki Ji" + name_hp = "Maharishi Valmiki's Birthday" dts = ( "2020-10-31", "2021-10-20", "2022-10-09", "2023-10-28", "2024-10-17", "2025-10-07", ) self._assertHinduHolidayHelper(name, dts, category_optional=True) # SUBDIVS. self._assertHinduHolidayHelper(name_pb, dts, subdivs={"PB"}) + self._assertHinduHolidayHelper(name_hp, dts, subdivs={"HP"})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/countries/test_india.py` around lines 794 - 807, Add a missing Himachal Pradesh coverage assertion in test_maharishi_valmiki_jayanti so the subdivision-specific holiday label "Maharishi Valmiki's Birthday" is tested alongside the existing country-wide and PB variants. Use the existing _assertHinduHolidayHelper helper in the test class to assert the same dts with subdivs={"HP"}, keeping the current name_pb PB assertion intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@holidays/countries/india.py`:
- Around line 604-608: In `_populate_subdiv_la_public_holidays` for Ladakh,
replace the obsolete `_add_nauroz(tr("Nauroz"))` call because `_add_nauroz` no
longer exists and will break `India(subdiv="LA")`. Use the same helper as the JK
subdivision path by calling `_add_nowruz_day(tr("Nauroz"))` so Ladakh holiday
population follows the current API.
In `@tests/countries/test_india.py`:
- Around line 1042-1057: The India holiday hook is still referencing a missing
Nauroz helper, causing the Ladakh subdivision path in India to fail. Update the
India holiday registration logic in the class/method that adds subdivision
holidays to call _add_nowruz_day instead of _add_nauroz, and make sure the LA
branch continues to use the correct subdivision-specific holiday helper
alongside the existing JK handling.
---
Outside diff comments:
In `@holidays/countries/india.py`:
- Around line 579-581: The Guru Arjun Dev Martyrdom Day holiday names are
inconsistent between JK and LA, causing the shared test expectation to fail.
Update the relevant holiday registration in the India holiday helpers,
especially _populate_subdiv_jk_optional_holidays and the matching LA subdivision
method, so both use the same capitalization for the holiday name. Keep the
string aligned with the shared assertion in
tests/countries/test_india.py::test_guru_arjun_dev_martyrdom_day.
In `@tests/countries/test_india.py`:
- Around line 794-807: Add a missing Himachal Pradesh coverage assertion in
test_maharishi_valmiki_jayanti so the subdivision-specific holiday label
"Maharishi Valmiki's Birthday" is tested alongside the existing country-wide and
PB variants. Use the existing _assertHinduHolidayHelper helper in the test class
to assert the same dts with subdivs={"HP"}, keeping the current name_pb PB
assertion intact.
🪄 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: d1213a58-483f-4110-b343-9f47b972e594
📒 Files selected for processing (3)
holidays/countries/india.pyholidays/groups/hindu.pytests/countries/test_india.py
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="holidays/locale/hi/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/hi/LC_MESSAGES/IN.po:326">
P2: Several newly added holiday entries in the Hindi locale file (`holidays/locale/hi/LC_MESSAGES/IN.po`) are left with empty `msgstr ""` values. Because GNU gettext falls back to the source `msgid` when a translation is empty, users with the `hi` locale will see English names for those holidays, producing a mixed Hindi/English experience. Since every pre-existing entry in this file is already fully translated, the blank entries appear to be an oversight rather than an intentional convention. Please add the corresponding Hindi translations for each new `msgid` so the locale remains complete and consistent.</violation>
</file>
<file name="holidays/locale/mr/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/mr/LC_MESSAGES/IN.po:328">
P2: The newly added Marathi `.po` entries for `Guru Ravidas's Jayanti`, `Karwa Chouth`, and `Maharishi Valmiki's Birthday` are untranslated aliases of already-translated holidays (`Guru Ravi Das's Jayanti`, `Karaka Chaturthi (Karwa Chouth)`, and `Maharishi Valmiki's Jayanti`). Since different Indian subdivisions emit these different msgids for the same underlying holiday, users will see English fallback in some states and Marathi translations in others, creating an inconsistent localization experience. Consider either reusing the existing msgids in the source Python code (avoiding alias proliferation that the project convention warns against) or copying the equivalent Marathi translations into the new entries.</violation>
</file>
<file name="holidays/locale/ml/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/ml/LC_MESSAGES/IN.po:328">
P2: The newly added holiday label `Guru Ravidas's Jayanti` is a variant spelling of the existing `Guru Ravi Das's Jayanti`, which already has a Malayalam translation in this file. Because both labels map to the same underlying holiday method in the source code (`_add_guru_ravidas_jayanti`), users in one subdivision will see Malayalam while users in another will see untranslated English. This fragments localization and creates maintenance drift—each variant must be translated separately even though it refers to the same observance. Prefer a single canonical English label in the source `.py` for each holiday so the generated `.po` can reuse one translated entry across all subdivisions.</violation>
</file>
<file name="holidays/locale/en_IN/LC_MESSAGES/IN.po">
<violation number="1" location="holidays/locale/en_IN/LC_MESSAGES/IN.po:444">
P2: Getexact `msgid` lookup means any typo or wording mismatch between a `.po` entry and the source `tr()` string creates a dead translation. Several newly added `msgid`s in this file do not match the literals used in `holidays/countries/india.py`:
- `International Women Day` should be `International Women's Day` (source: `india.py:669`).
- `Birthday of Sri Guru Nabha Dass ji` should be `Birthday of Sri Guru Nabha Dass Ji` (source: `india.py:646`).
- `Martyrdom Day of S. Kartar Singh Sarabha ji` should be `Martyrdom Day of S. Kartar Singh Sarabha Ji` (source: `india.py:658`).
- `Martyrdom Day of Sri Guru Teg Bahadur ji` should be `Martyrdom Day of Sri Guru Teg Bahadur Ji` (source: `india.py:661`).
Because the same incorrect `msgid`s are present in all India locale files, any future or existing translations for those strings will never be resolved at runtime. Please regenerate the `.po` files with `make l10n` or correct the `msgid`s to exactly match the source strings.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
arkid15r
reviewed
Jul 29, 2026
|
KJhellico
previously approved these changes
Jul 29, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
holidays/countries/india.py (1)
542-543: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore Delhi’s public Chhath Puja.
Replacing the Delhi populator with
passremoves the subdivision-specific publicChhath Puja. The national optional holiday has a different name and category, soIndia(subdiv="DL")no longer exposes Delhi’s public holiday.🛠️ Proposed fix
# Delhi. def _populate_subdiv_dl_public_holidays(self): - pass + # Chhath Puja. + self._add_chhath_puja(tr("Chhath Puja"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@holidays/countries/india.py` around lines 542 - 543, Implement _populate_subdiv_dl_public_holidays to add Delhi’s subdivision-specific public Chhath Puja holiday, restoring the existing date and holiday naming conventions used by the India calendar while keeping it distinct from the national optional holiday.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@holidays/countries/india.py`:
- Around line 542-543: Implement _populate_subdiv_dl_public_holidays to add
Delhi’s subdivision-specific public Chhath Puja holiday, restoring the existing
date and holiday naming conventions used by the India calendar while keeping it
distinct from the national optional holiday.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 451f5327-b506-4376-9d38-133d8806aa4a
📒 Files selected for processing (3)
holidays/constants.pyholidays/countries/india.pytests/countries/test_india.py
PPsyrius
reviewed
Jul 30, 2026
KJhellico
reviewed
Jul 30, 2026
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Ankush Kapoor <work.ankushkapoor1626@gmail.com>
|
PPsyrius
approved these changes
Jul 30, 2026
KJhellico
approved these changes
Jul 30, 2026
arkid15r
enabled auto-merge
July 30, 2026 15:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Fix subdiv holidays of India (North region).
Fixes -
Chandigarh
Haryana
Himachal Pradesh
Jammu and Kashmir
Ladakh
Punjab
Uttarakhand
Note - National holidays were added from holidays list observed in Delhi and therefore, Delhi subdiv is already up to date and fixed.
Resources:
Official Notices collection
Part of GSoC and #3382
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.