Update India holidays: add Maharishi Valmiki Jayanti holiday#3485
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:
WalkthroughAdds the optional Hindu holiday "Maharishi Valmiki Jayanti": new holiday constant and per‑year estimated lunisolar mapping with accessor; wired into Hindu group and India optional registration; IN locale translations and tests added; contributor entry added. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Actionable comments posted: 4
🤖 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 517-518: India._populate_subdiv_up_public_holidays() is
redundantly calling self._add_maharishi_valmiki_jayanti(...) even though
India._populate_public_holidays() already adds Maharishi Valmiki Jayanti for all
subdivisions; remove the duplicate call to _add_maharishi_valmiki_jayanti from
India._populate_subdiv_up_public_holidays() so the holiday is only added once
via India._populate_public_holidays(), avoiding duplicate entries and merge
ambiguity.
- Line 199: Update the three comment lines reading "# Maharishi Valmiki Jayanti"
in holidays/countries/india.py so they include a trailing period — change each
to "# Maharishi Valmiki Jayanti." (the occurrences referenced are the comment
text "Maharishi Valmiki Jayanti" at the three noted locations, also listed as
lines 199, 266 and 517 in the review) to normalize comment style and ensure
generated en_US translator comments end with a period.
In `@holidays/locale/en_IN/LC_MESSAGES/IN.po`:
- Around line 52-55: The msgstr for the translation entry with msgid "Maharishi
Valmiki Jayanti" should be left empty to follow the source-language fallback
pattern used by Lingva; edit the entry so the msgstr is an empty string (i.e.,
replace the literal value currently set for the msgid "Maharishi Valmiki
Jayanti" with an empty msgstr) so the system falls back to the msgid.
In `@holidays/locale/en_US/LC_MESSAGES/IN.po`:
- Line 52: The translator comment "Maharishi Valmiki Jayanti" in the generated
en_US PO must end with a period; fix the source comment in
holidays/countries/india.py (the comment that documents the "Maharishi Valmiki
Jayanti" holiday) by adding a trailing period, then regenerate localization
files (run make l10n) so the .po file is updated rather than editing IN.po
directly.
🪄 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: 857b66dd-4ec5-4b5c-a88e-56d68b2ee932
📒 Files selected for processing (11)
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/ta/LC_MESSAGES/IN.poholidays/locale/te/LC_MESSAGES/IN.potests/countries/test_india.py
KJhellico
reviewed
Apr 1, 2026
Collaborator
|
Also you need to merge latest |
Collaborator
|
@vanshika05-ai, are you planning to finish this PR? |
Contributor
Author
|
@KJhellico I’m working on it |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
holidays/locale/en_IN/LC_MESSAGES/IN.po (1)
52-54:⚠️ Potential issue | 🟡 Minormsgstr should be empty for the source language.
Line 54 contains a literal value when the en_IN locale (India's default language) should use empty msgstr to rely on fallback behavior. Based on learnings, source/default language files should leave msgstr empty.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holidays/locale/en_IN/LC_MESSAGES/IN.po` around lines 52 - 54, The msgstr for the entry with msgid "Maharishi Valmiki Jayanti" should be empty for the source/default language; replace the current literal msgstr value with an empty string (msgstr "") so the en_IN locale relies on fallback behavior—locate the msgid "Maharishi Valmiki Jayanti" and set its msgstr to "".holidays/countries/india.py (1)
207-209:⚠️ Potential issue | 🟠 MajorRemove from PUBLIC holidays; keep as OPTIONAL only.
Per maintainer feedback, Maharishi Valmiki Jayanti is a Restricted holiday at the national level. It should only be added in
_populate_optional_holidays(), not in_populate_public_holidays().Suggested fix
- # Maharishi Valmiki Jayanti - self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holidays/countries/india.py` around lines 207 - 209, Remove the call to self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) from the _populate_public_holidays() implementation and instead call self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) inside _populate_optional_holidays(); reference the same helper method name (_add_maharishi_valmiki_jayanti) and keep the tr(...) translation wrapper intact so the holiday is only registered as OPTIONAL, not PUBLIC.
🤖 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/calendars/hindu.py`:
- Around line 1561-1563: Replace the string literal "MAHARISHI_VALMIKI_JAYANTI"
in the maharishi_valmiki_jayanti_date method with the existing constant
MAHARISHI_VALMIKI_JAYANTI (use the constant for consistency with other holiday
getters), and relocate the maharishi_valmiki_jayanti_date method so it is
alphabetically ordered among holiday methods—place it between maha_navami_date
and maha_shivaratri_date (or immediately after maharana_pratap_jayanti_date if
that matches the repository's sort) to maintain the file's method ordering
convention.
In `@tests/countries/test_india.py`:
- Line 48: The test currently expects "2018-10-24" in the PUBLIC holidays list;
if Maharishi Valmiki Jayanti is removed from _populate_public_holidays() or
changed to OPTIONAL, update the test expectations accordingly by removing
"2018-10-24" from the PUBLIC/expected public-holidays list (or moving it into
the OPTIONAL/expected optional-holidays list) in tests/countries/test_india.py
so the test aligns with the updated _populate_public_holidays() behavior.
---
Duplicate comments:
In `@holidays/countries/india.py`:
- Around line 207-209: Remove the call to
self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) from the
_populate_public_holidays() implementation and instead call
self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) inside
_populate_optional_holidays(); reference the same helper method name
(_add_maharishi_valmiki_jayanti) and keep the tr(...) translation wrapper intact
so the holiday is only registered as OPTIONAL, not PUBLIC.
In `@holidays/locale/en_IN/LC_MESSAGES/IN.po`:
- Around line 52-54: The msgstr for the entry with msgid "Maharishi Valmiki
Jayanti" should be empty for the source/default language; replace the current
literal msgstr value with an empty string (msgstr "") so the en_IN locale relies
on fallback behavior—locate the msgid "Maharishi Valmiki Jayanti" and set its
msgstr to "".
🪄 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: be0fec8c-a801-463a-9983-b8721bca7fc1
📒 Files selected for processing (11)
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/ta/LC_MESSAGES/IN.poholidays/locale/te/LC_MESSAGES/IN.potests/countries/test_india.py
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
holidays/countries/india.py (2)
278-278:⚠️ Potential issue | 🟡 MinorAdd trailing periods to the l10n comments.
These source comments generate translator comments, so use the standard
# Maharishi Valmiki Jayanti.form.Suggested cleanup
- # Maharishi Valmiki Jayanti + # Maharishi Valmiki Jayanti. self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti"))- # Maharishi Valmiki Jayanti + # Maharishi Valmiki Jayanti. self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti"))Based on learnings: holiday-name comments directly above holiday addition methods must match the holiday name exactly and message comments should end with a period.
Also applies to: 550-550
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holidays/countries/india.py` at line 278, Update the translator-facing comment(s) so they end with a period and exactly match the holiday name; e.g., change the comment above the Maharishi Valmiki Jayanti holiday addition to "# Maharishi Valmiki Jayanti." and make the same fix for the duplicate occurrence around line 550. Ensure the comment directly above the holiday addition method matches the holiday name exactly and ends with a trailing period so l10n tooling generates the correct translator comment.
210-211:⚠️ Potential issue | 🟠 MajorKeep this out of national PUBLIC holidays.
Maharishi Valmiki Jayanti was called out as a Restricted holiday at the national level, so adding it in
_populate_public_holidays()makes it PUBLIC for every India instance/subdivision. Keep the national entry in_populate_optional_holidays()instead; if UP/DL are public by state source, add only those subdivision-specific entries.Suggested cleanup
- # Maharishi Valmiki Jayanti - self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) - # Diwali. self._add_diwali_india(tr("Diwali"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@holidays/countries/india.py` around lines 210 - 211, The call to add Maharishi Valmiki Jayanti in _populate_public_holidays() makes it a national PUBLIC holiday; instead remove or relocate the call to _populate_optional_holidays() so it remains a restricted/optional national holiday. Specifically, delete or comment out the line invoking self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki Jayanti")) from _populate_public_holidays(), and add the same invocation into _populate_optional_holidays(); if any state-specific logic (e.g., Uttar Pradesh/Delhi) requires it as public, only add those subdivision-specific entries in the state population code rather than the national public list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/countries/test_india.py`:
- Around line 490-497: The dt tuple in the test for "Maharishi Valmiki Jayanti"
contains a non-ISO zero-padded date "2025-10-7"; update the fixture in the dt
tuple to use the zero-padded form "2025-10-07" so all entries are ISO-formatted
and the assertion in self.assertHolidayName(name, opt_holidays, dt) is
consistent.
- Around line 23-29: Replace the broad warnings.simplefilter("ignore") calls in
setUpClass and setUp with a narrow filter that only ignores the specific Hindu
out-of-range UserWarning: call warnings.filterwarnings("ignore",
category=UserWarning, message=".*Hindu.*out[- ]?of[- ]?range.*") inside
setUpClass(cls) and setUp(self) so only the intended UserWarning is suppressed
while other warnings remain visible; reference the setUpClass and setUp methods
and the warnings.simplefilter usage when making the change.
---
Duplicate comments:
In `@holidays/countries/india.py`:
- Line 278: Update the translator-facing comment(s) so they end with a period
and exactly match the holiday name; e.g., change the comment above the Maharishi
Valmiki Jayanti holiday addition to "# Maharishi Valmiki Jayanti." and make the
same fix for the duplicate occurrence around line 550. Ensure the comment
directly above the holiday addition method matches the holiday name exactly and
ends with a trailing period so l10n tooling generates the correct translator
comment.
- Around line 210-211: The call to add Maharishi Valmiki Jayanti in
_populate_public_holidays() makes it a national PUBLIC holiday; instead remove
or relocate the call to _populate_optional_holidays() so it remains a
restricted/optional national holiday. Specifically, delete or comment out the
line invoking self._add_maharishi_valmiki_jayanti(tr("Maharishi Valmiki
Jayanti")) from _populate_public_holidays(), and add the same invocation into
_populate_optional_holidays(); if any state-specific logic (e.g., Uttar
Pradesh/Delhi) requires it as public, only add those subdivision-specific
entries in the state population code rather than the national public list.
🪄 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: 90e4029b-c20a-4117-8a7d-706d419cf8b9
📒 Files selected for processing (3)
holidays/countries/india.pyholidays/groups/hindu.pytests/countries/test_india.py
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
auto-merge was automatically disabled
April 30, 2026 05:15
Head branch was pushed to by a user without write access
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
Signed-off-by: Vanshika <pundirvanshika9@gmail.com>
PPsyrius
previously approved these changes
Apr 30, 2026
|
KJhellico
previously approved these changes
May 4, 2026
|
Contributor
There was a problem hiding this comment.
No issues found across 16 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
arkid15r
approved these changes
May 4, 2026
Merged
9 tasks
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
4 participants
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.
Add Maharishi Valmiki Jayanti to the India holiday calendar.
Changes
Implementation Notes
Testing
pytestDesign Consideration
Maharishi Valmiki Jayanti is observed as a public holiday in certain Indian states (e.g., Delhi and Uttar Pradesh) and as an optional holiday more broadly.
This PR reflects that by:
If the project prefers restricting it to optional holidays only, I’m happy to adjust the implementation accordingly.
Happy to refine this further based on feedback. Thanks for reviewing!
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.