Add Hindu calendar generator script - #3563
Open
ankushhKapoor wants to merge 69 commits into
Open
Conversation
currently includes imp astronomical calculations for 9 major holidays
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 a standalone Hindu lunisolar and solar holiday generator using pyephem: astronomy helpers, tithi/day‑part primitives, lunisolar holiday resolvers, a solar Makar Sankranti resolver, registries, and codegen for years 2001–2100. ChangesHindu Holiday Date Generator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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: 7
🤖 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 `@scripts/calendar/hindu_generator.py`:
- Line 33: Fix the typo in the inline comment next to the
swe.set_sid_mode(swe.SIDM_LAHIRI) call: change "gloablly" to "globally" so the
comment reads "set Lahiri ayanamsa globally".
- Line 694: Fix the typo in the event key by replacing the string "JANMASTHAMI"
with the correct "JANMASHTAMI" in the mapping tuple that pairs the event name to
the function get_janmashtami; search for other occurrences of the misspelled
constant (e.g., any code that reads calendar keys or writes generated calendar
files) and update them to the corrected spelling to avoid propagating the typo
to outputs and tests.
- Around line 690-700: The HINDU_HOLIDAYS tuple uses unnecessary lambda wrappers
that just forward their single argument; replace each pair like ("DIWALI_INDIA",
lambda y: get_diwali(y)) with ("DIWALI_INDIA", get_diwali) (do the same for
DUSSEHRA, HOLI, JANMASTHAMI, MAHA_SHIVARATRI, GANESH_CHATURTHI,
GURU_NANAK_JAYANTI, RAM_NAVAMI, SHARAD_NAVRATRI) so the functions themselves are
stored as callables; ensure HINDU_HOLIDAYS remains a tuple of (name, function)
pairs and that any code invoking these entries still calls them with the year
argument.
- Line 712: Replace the locale-dependent dt.strftime call when building date_str
with a static MONTHS lookup: define MONTHS =
("JAN","FEB","MAR","APR","MAY","JUN","JUL","AUG","SEP","OCT","NOV","DEC") near
the top of the module and change the date_str assignment that currently uses
dt.strftime('%b').upper() to use MONTHS[dt.month - 1] (keep the ", {dt.day}"
part unchanged); update any other similar occurrences of dt.strftime('%b') in
this file to use the MONTHS tuple as well.
- Around line 633-638: The loop currently requires sign_prev == 5 for both
branches, which wrongly skips the case where t == 30 and sign == 5; change the
conditional so the t == 30 && sign == 5 branch sets ashwin_ama immediately (no
sign_prev check), while the t == 1 && t_prev == 29 && sign == 5 branch still
verifies sign_prev == 5; locate the check around variables t, t_prev, sign,
sign_prev and functions sidereal_solar_zodiac_sign/sunset_jd and split the
combined if into two branches accordingly.
- Around line 236-238: The code assumes bhadra_ama is set before using it (dt =
bhadra_ama + timedelta(...)) which will raise if bhadra_ama is None; add a guard
right before that line to check if bhadra_ama is None and handle it (e.g., raise
a descriptive exception, return/skip from the containing function, or compute an
alternative) so the loop that uses bhadra_ama (the dt = bhadra_ama +
timedelta(days=1) and while dt <= bhadra_ama + timedelta(days=10) block) never
executes with a None value; reference the bhadra_ama variable and the loop that
initializes dt to locate where to add the check.
- Around line 502-515: The current return at the end always returns
phalgun_chaturdashi which can be None even when magh_chaturdashi is available;
update the logic so that if phalgun_chaturdashi is None but magh_chaturdashi
exists, the function returns magh_chaturdashi as a fallback. Concretely, after
the existing conditional block that checks both phalgun_chaturdashi and
magh_chaturdashi, add a check: if phalgun_chaturdashi is None and
magh_chaturdashi is not None, return magh_chaturdashi; otherwise keep the
current return of phalgun_chaturdashi. Ensure you reference the variables
phalgun_chaturdashi and magh_chaturdashi (and keep existing special-case checks
for p/m) so behavior for dual-present cases is unchanged.
🪄 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: 1fe731c3-0f60-437c-871c-73cbbde989b2
📒 Files selected for processing (1)
scripts/calendar/hindu_generator.py
Contributor
Author
|
Most of the code was written iteratively (trial and error) by finding common patterns across several years. In some cases exceptions had to be used (I didn't want to but couldn't find any consistent pattern for those years) |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 1 file
Confidence score: 2/5
- Several concrete issues in
scripts/calendar/hindu_generator.pyare likely to cause user-visible calendar inaccuracies (for example,get_holi()returning Holika Dahan instead of next-day Holi, and Ashwin Amavasya logic rejecting validt == 30cases at sign ingress). - There is a real runtime stability risk: adding
timedeltawhenbhadra_amais missing can raiseTypeError, and the undeclaredswissephdependency can make the generator fail to import in a standard environment. - Locale-dependent
%bmonth generation and the non-canonicalJANMASHTAMIkey add consistency risks in generated outputs, so this is more than housekeeping despite being fixable. - Pay close attention to
scripts/calendar/hindu_generator.py- date-rule correctness, null-guarding, and deterministic code generation need fixes before merge.
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="scripts/calendar/hindu_generator.py">
<violation number="1" location="scripts/calendar/hindu_generator.py:18">
P2: This script depends on `swisseph`, but that dependency is not declared in the project, so the generator fails to import in a standard repo environment.</violation>
<violation number="2" location="scripts/calendar/hindu_generator.py:237">
P1: Guard against a missing `bhadra_ama` before adding `timedelta`; otherwise this path can raise a `TypeError` and stop generation for that year.</violation>
<violation number="3" location="scripts/calendar/hindu_generator.py:371">
P1: `get_holi()` returns Holika Dahan dates in this block instead of advancing to Holi (the next day), causing an off-by-one result vs its documented rule.</violation>
<violation number="4" location="scripts/calendar/hindu_generator.py:633">
P2: The Ashwin Amavasya detection incorrectly requires the previous day to also be in sign 5, which can reject valid `t == 30` matches at sign ingress.</violation>
<violation number="5" location="scripts/calendar/hindu_generator.py:694">
P2: Use the canonical `JANMASHTAMI` identifier to avoid generating a mismatched holiday key.</violation>
<violation number="6" location="scripts/calendar/hindu_generator.py:712">
P1: Avoid locale-dependent `%b` when generating month constants; this can emit non-English abbreviations and produce inconsistent or invalid generated calendar code.</violation>
</file>
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.
Fix all with cubic.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@scripts/calendar/hindu_generator.py`:
- Around line 680-688: The generated module omits month constant definitions so
YEAR_TEMPLATE entries like (MAR, 14) reference undefined symbols; update the
generator (in hindu_generator.py) so CLASS_TEMPLATE output includes month
constant imports or definitions before the class body and ensure YEAR_TEMPLATE
emits either numeric month values or the same constant names used in the import;
specifically modify CLASS_TEMPLATE and/or the top of the generated file to
include a line importing or defining JAN..DEC (or change YEAR_TEMPLATE to use
integers) so the produced holidays/calendars/hindu_dates.py no longer references
undefined month symbols.
- Around line 370-380: The get_holi() helper is returning Holika Dahan dates
instead of Holi; update the three branches that handle Purnima transitions so
they return the Holi date: when t == 16 and t_prev == 14 (Purnima skipped/night
of Holika Dahan) and when t == 16 and t_prev == 16 (Purnima spanned 2 sunsets)
return dt + timedelta(days=1) so HOLI is the following day, and when t > 16 and
t_prev == 16 (only one sunset had Purnima) return dt (the current day) instead
of dt - timedelta(days=1); refer to the get_holi function and the variables t,
t_prev, dt to locate and modify the returns accordingly.
🪄 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: d28c66ef-dc40-440b-9169-1f4fe3eece5f
📒 Files selected for processing (1)
scripts/calendar/hindu_generator.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3563 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 329 329
Lines 19766 20011 +245
Branches 2508 2509 +1
==========================================
+ Hits 19766 20011 +245 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PPsyrius
reviewed
May 5, 2026
made helper fns private removed unnecessary lambda use
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
scripts/calendar/hindu_generator.py (1)
761-764:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLocale-dependent month formatting still pending — please apply the static
MONTHStuple fix.
dt.strftime('%b').upper()emits non-English month abbreviations under non-English locales, which corrupts the generatedhindu_dates.py. The Mongolian script having the same bug isn't a precedent; the canonical pattern in this repo ishebrew_generator.py's staticMONTHStuple.🛠️ Suggested change
+MONTHS = ("JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC") + # Coordinates for Ujjain, India (holy city used in Hindu astrology) LAT = "23.1765"- if dt: - date_str = f"{dt.strftime('%b').upper()}, {dt.day}" - else: - date_str = "None" + date_str = f"{MONTHS[dt.month - 1]}, {dt.day}" if dt else "None"Based on learnings: in this repository's generator scripts under
scripts/calendar/, follow the established convention of using a staticMONTHStuple to generate uppercase English month abbreviations and avoidstrftime('%b')because it is locale-dependent.🤖 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 `@scripts/calendar/hindu_generator.py` around lines 761 - 764, Replace the locale-dependent dt.strftime('%b').upper() usage with the repository pattern of a static MONTHS tuple: ensure a MONTHS tuple of English 3-letter uppercase abbreviations exists in hindu_generator.py (same form as in hebrew_generator.py), then build date_str using MONTHS[dt.month - 1] when dt is truthy (keep the "None" fallback when dt is falsy); update the code that sets date_str (the block referencing dt and date_str) to use the MONTHS lookup instead of strftime so generated hindu_dates.py is locale-independent.
🤖 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 `@pyproject.toml`:
- Line 53: The pyproject.toml dev dependency "pyephem>=9.99" lacks an upper
bound; update the pyephem entry (the string "pyephem>=9.99") to follow the
project's pinning convention by adding an upper version cap (e.g., change to a
range like "pyephem>=9.99,<10") so future major releases are excluded.
In `@scripts/calendar/hindu_generator.py`:
- Line 141: There's a typo in the month/const list entry containing the string
"Jyeshth" where the comment reads "Tauras" — update the comment to "Taurus" (fix
the misspelling) in the same list/array in scripts/calendar/hindu_generator.py
so the entry for "Jyeshth" reads "... - Taurus (1)"; locate the "Jyeshth" string
literal in the month names/constants and correct the comment text.
- Around line 155-157: Add explicit type annotations to the _lunar_month helper
to satisfy Ruff ANN202 and match other helpers: change the signature to def
_lunar_month(jd: float) -> str: (keep the body using sign =
_sidereal_solar_zodiac_sign(jd) and return MONTH_NAMES[sign]) so the parameter
and return types are declared consistently with functions like _norm360 and
_tithi.
- Around line 696-720: The three-iteration search window (for _ in range(3))
that calls _sunrise_jd and _tithi can miss edge-case tithi transitions and
currently returns None silently; widen the search window (e.g., change range(3)
to range(5) or to the same 10–20 day window used by other finder functions) so
Pratipada detection (the branches using t == 30, t == 1, and t == 2 and t_prev
== 30) has a defensive fallback, and keep or update the comment to reflect the
new window length so the function no longer risks falling through to None
unexpectedly.
- Around line 21-27: The two triple-quoted source/credits blocks in
scripts/calendar/hindu_generator.py are currently placed after imports and thus
are not recognized as the module docstring; move the primary sources/credits
triple-quoted string so it is the very first statement in the file (above all
imports) to become the module docstring, and convert the second triple-quoted
block (the later block currently inside the file) into a normal comment or doc
comment adjacent to the related functions/classes so it isn't a discarded string
literal; look for the top-level source block and the later block in the file to
relocate/convert accordingly.
- Around line 80-91: The sunrise/sunset limb conventions are inconsistent:
_sunrise_jd uses use_center=False (upper-limb) while _sunset_jd uses
use_center=True (disc-centre). In _sunset_jd (and its docstring) change the
ephem.next_setting call to use_center=False and update the docstring to state it
returns the JD of sunset at Ujjain for the Sun's upper limb so both functions
use the same limb convention (upper-limb) to avoid asymmetric day-length and
timing shifts.
---
Duplicate comments:
In `@scripts/calendar/hindu_generator.py`:
- Around line 761-764: Replace the locale-dependent dt.strftime('%b').upper()
usage with the repository pattern of a static MONTHS tuple: ensure a MONTHS
tuple of English 3-letter uppercase abbreviations exists in hindu_generator.py
(same form as in hebrew_generator.py), then build date_str using MONTHS[dt.month
- 1] when dt is truthy (keep the "None" fallback when dt is falsy); update the
code that sets date_str (the block referencing dt and date_str) to use the
MONTHS lookup instead of strftime so generated hindu_dates.py is
locale-independent.
🪄 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: 095c0d8b-6fc8-4ac4-9382-b300f56da828
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
pyproject.tomlscripts/calendar/hindu_generator.py
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Ankush Kapoor <work.ankushkapoor1626@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/calendar/hindu_generator.py (1)
274-280:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSkipped-Amavasya branch missing
sign_prevcheck (inconsistent with other finders).
get_dussehra(line 220-224) andget_sharad_navratri(line 683-687) verifysign_prev == <target_sign>before accepting thet == 1 and t_prev == 29skipped-Amavasya case. The same branch here, inget_guru_nanak_jayanti(line 323),get_holi(lines 388, 392), andget_janmashtami(line 462), accepts the skip without that check.If the Sun crosses into the target sign on the very day where
t == 1 and t_prev == 29, the previous day was actually a different lunar month's Amavasya, and these finders will lock onto the wrong reference point. The author's verification against existing data may not trigger this edge case for 2001–2100, but the inconsistency is fragile.♻️ Suggested fix (apply analogously in the four other finders)
- if (t == 30 and sign == 4) or (t == 1 and t_prev == 29 and sign == 4): - bhadra_ama = dt + if t == 30 and sign == 4: + bhadra_ama = dt + elif t == 1 and t_prev == 29 and sign == 4: + sign_prev = _sidereal_solar_zodiac_sign(_sunset_jd(dt - timedelta(days=1))) + if sign_prev == 4: + bhadra_ama = dt🤖 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 `@scripts/calendar/hindu_generator.py` around lines 274 - 280, The skipped-Amavasya branch in the bhadra_ama search accepts the (t == 1 and t_prev == 29) case without verifying sign_prev, which can pick an Amavasya from the prior solar month; update the condition in the bhadra_ama assignment so the skipped-Amavasya arm also requires sign_prev == 4 (i.e. change the second part to "t == 1 and t_prev == 29 and sign == 4 and sign_prev == 4"). Apply the same pattern to the analogous checks in get_guru_nanak_jayanti, get_holi, and get_janmashtami: require sign_prev match the target sign in the (t == 1 and t_prev == 29) branch to ensure the previous day’s lunar month aligns with the expected solar sign.
🤖 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 `@scripts/calendar/hindu_generator.py`:
- Around line 736-750: The CALENDARS dict is unused; either remove it or use it
to drive generation — update the generator so class_name and OUT_FILE_NAME are
derived from CALENDARS instead of hard-coded strings (referencing CALENDARS,
class_name, OUT_FILE_NAME) or simply delete CALENDARS and any dead references;
ensure HINDU_HOLIDAYS remains intact and the rendering loop (where get_*
functions are invoked) uses the selected calendar name consistently.
---
Duplicate comments:
In `@scripts/calendar/hindu_generator.py`:
- Around line 274-280: The skipped-Amavasya branch in the bhadra_ama search
accepts the (t == 1 and t_prev == 29) case without verifying sign_prev, which
can pick an Amavasya from the prior solar month; update the condition in the
bhadra_ama assignment so the skipped-Amavasya arm also requires sign_prev == 4
(i.e. change the second part to "t == 1 and t_prev == 29 and sign == 4 and
sign_prev == 4"). Apply the same pattern to the analogous checks in
get_guru_nanak_jayanti, get_holi, and get_janmashtami: require sign_prev match
the target sign in the (t == 1 and t_prev == 29) branch to ensure the previous
day’s lunar month aligns with the expected solar sign.
🪄 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: e4608007-ebc8-4046-8968-17ca5a3b9402
📒 Files selected for processing (1)
scripts/calendar/hindu_generator.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
scripts/calendar/hindu_generator.py (2)
762-762:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace locale-dependent
strftime('%b')with a staticMONTHStuple.
dt.strftime('%b').upper()resolves through the active C locale and can emit non-English month abbreviations (e.g., "MAI" instead of "MAY"), which would corrupt the generatedhindu_dates.py. The repository convention forscripts/calendar/*generators is a staticMONTHStuple —hebrew_generator.pyuses this pattern.🩹 Proposed fix
Add near the other module-level constants (e.g., next to
MONTH_NAMES):MONTHS = ("JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC")Then update line 762:
- date_str = f"{dt.strftime('%b').upper()}, {dt.day}" + date_str = f"{MONTHS[dt.month - 1]}, {dt.day}"Based on learnings from
scripts/calendar/hebrew_generator.py: generator scripts underscripts/calendar/follow the convention of using a staticMONTHStuple to emit uppercase English month abbreviations, sincedt.strftime('%b')is locale-dependent.🤖 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 `@scripts/calendar/hindu_generator.py` at line 762, The code uses locale-dependent dt.strftime('%b').upper() when building date_str which can produce non-English month abbreviations; add a module-level MONTHS = ("JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG", "SEP", "OCT", "NOV", "DEC") next to existing constants like MONTH_NAMES and replace the dt.strftime('%b').upper() usage in the date_str construction (the line assigning date_str) with a lookup into MONTHS using dt.month-1 so the generator emits stable English month abbreviations (following the pattern used in hebrew_generator.py).
416-426:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
get_holi()returns Holika Dahan instead of Holi.The docstring on lines 360-372 explicitly states: "The day Purnima ends = Holika Dahan. Holi = next day." But the three return statements all yield the Holika Dahan day:
- Line 418 (
t==16,t_prev==14): returnsdt, which the comment itself labels "night of Holika Dahan".- Line 422 (
t==16,t_prev==16): returnsdt, which the comment labels "current day is Holika Dahan".- Line 426 (
t>16,t_prev==16): returnsdt - timedelta(days=1), which the comment labels "t=16 is Holika Dahan".In each branch the result needs to be advanced by one day to match the function name and the registry entry
("HOLI", get_holi)at line 743.🩹 Proposed fix
# Purnima skipped entirely between sunsets (14->16)- night of Holika Dahan if t == 16 and t_prev == 14: - return dt + return dt + timedelta(days=1) # Purnima spanned 2 sunsets, now fully ended (16->16) - current day is Holika Dahan if t == 16 and t_prev == 16: - return dt + return dt + timedelta(days=1) # Only 1 sunset had Purnima, next jumped past 16 (15->16->17)- t=16 is Holika Dahan if t > 16 and t_prev == 16: - return dt - timedelta(days=1) + return dt🤖 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 `@scripts/calendar/hindu_generator.py` around lines 416 - 426, In get_holi(), the branches that detect Purnima/Holika Dahan (conditions using t, t_prev) currently return the Holika Dahan date; change each of the three return values to return the following day instead: replace the returns in the t==16 && t_prev==14 branch, the t==16 && t_prev==16 branch, and the t>16 && t_prev==16 branch so they return dt + timedelta(days=1) (use the existing timedelta import), ensuring the function (and the registry entry ("HOLI", get_holi)) yields Holi, the day after Holika Dahan.
🤖 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.
Duplicate comments:
In `@scripts/calendar/hindu_generator.py`:
- Line 762: The code uses locale-dependent dt.strftime('%b').upper() when
building date_str which can produce non-English month abbreviations; add a
module-level MONTHS = ("JAN", "FEB", "MAR", "APR", "MAY", "JUN", "JUL", "AUG",
"SEP", "OCT", "NOV", "DEC") next to existing constants like MONTH_NAMES and
replace the dt.strftime('%b').upper() usage in the date_str construction (the
line assigning date_str) with a lookup into MONTHS using dt.month-1 so the
generator emits stable English month abbreviations (following the pattern used
in hebrew_generator.py).
- Around line 416-426: In get_holi(), the branches that detect Purnima/Holika
Dahan (conditions using t, t_prev) currently return the Holika Dahan date;
change each of the three return values to return the following day instead:
replace the returns in the t==16 && t_prev==14 branch, the t==16 && t_prev==16
branch, and the t>16 && t_prev==16 branch so they return dt + timedelta(days=1)
(use the existing timedelta import), ensuring the function (and the registry
entry ("HOLI", get_holi)) yields Holi, the day after Holika Dahan.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f7977ec7-d0a8-438d-b370-2433a943935e
📒 Files selected for processing (1)
scripts/calendar/hindu_generator.py
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (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="scripts/calendar/hindu_generator.py">
<violation number="1" location="scripts/calendar/hindu_generator.py:357">
P2: Docstring is self-referential: "Holi...the day after Holi" should be "the day after Holika Dahan" (as the original stated and as the subsequent line in this same docstring confirms).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic.
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[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>
KJhellico
reviewed
May 6, 2026
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (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="pyproject.toml">
<violation number="1" location="pyproject.toml:53">
P1: The new version constraint is unsatisfiable: `pyephem` has no 4.x releases, so installing the `dev` dependency group will fail.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic.
Collaborator
|
I recommend moving all the calendar calculation logic into a separate class (as in |
9 tasks
use class attributes, static methods, cache methods
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 3/5
- In
scripts/calendar/hindu_generator.py, Adhik Ashwin handling selects the first Virgo Amavasya instead of the normal-month Mahalaya Amavasya, producing an incorrect date such as September 17 instead of October 16; select the last Amavasya in the relevant sequence and add regression coverage for 2020.
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="scripts/calendar/hindu_generator.py">
<violation number="1" location="scripts/calendar/hindu_generator.py:304">
P1: In Adhik Ashwin years such as 2020, this returns the first Virgo Amavasya instead of the normal-month Mahalaya Amavasya, producing September 17 rather than the expected October 16. Request the last Amavasya in the sign before applying the boundary guard.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| return exceptions[year] | ||
|
|
||
| # Find Mahalaya Amavasya (Ashwin Amavasya). | ||
| return self._get_amavasya(date(year, 9, 1), zodiac_sign=5, sign_boundary_guard=True) |
Contributor
There was a problem hiding this comment.
P1: In Adhik Ashwin years such as 2020, this returns the first Virgo Amavasya instead of the normal-month Mahalaya Amavasya, producing September 17 rather than the expected October 16. Request the last Amavasya in the sign before applying the boundary guard.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/calendar/hindu_generator.py, line 304:
<comment>In Adhik Ashwin years such as 2020, this returns the first Virgo Amavasya instead of the normal-month Mahalaya Amavasya, producing September 17 rather than the expected October 16. Request the last Amavasya in the sign before applying the boundary guard.</comment>
<file context>
@@ -286,6 +286,44 @@ def get_basant_panchami(self, year: int) -> date | None:
+ return exceptions[year]
+
+ # Find Mahalaya Amavasya (Ashwin Amavasya).
+ return self._get_amavasya(date(year, 9, 1), zodiac_sign=5, sign_boundary_guard=True)
+
+ def get_bonalu(self, year: int) -> date | None:
</file context>
Suggested change
| return self._get_amavasya(date(year, 9, 1), zodiac_sign=5, sign_boundary_guard=True) | |
| return self._get_amavasya(date(year, 9, 1), zodiac_sign=5, last=True, sign_boundary_guard=True) |
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
Added Hindu calendar generator script based on astronomical calculations (tithi, sunrise/sunset).
Part of GSoC and #3382
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.