Refactor calendar generators#3565
Merged
Merged
Conversation
Contributor
Summary by CodeRabbit
WalkthroughRefactors calendar generators to use a centralized CalendarGenerator; updates lunisolar data and Hindu calendar lookup tables (Diwali, Thaipusam); extends Malaysia Thaipusam year entries; regenerates many country snapshot JSONs and updates a few tests to match recalculated dates. ChangesGenerator infrastructure refactor
Hindu calendar data, snapshots, and tests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3565 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 314 314
Lines 18766 18766
Branches 2401 2401
=========================================
Hits 18766 18766 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/asian_generator.py`:
- Around line 317-330: thaipusam_date can underflow to year 1900 and call
_get_leap_month(1900) which indexes the wrong lunar table; prevent this by
guarding the lower bound before subtracting one year (special-case the first
supported year 1901). In thaipusam_date, ensure y is not set below 1901 (e.g. if
year == 1901 keep y = 1901 or use y = max(year - 1, 1901)) so that the
subsequent call to _get_leap_month(y) and lunar_to_gre(y, m, 15) only receive
supported years; alternatively extend the lunar table to include 1900, but
prefer the simple lower-bound check to avoid indexing G_LUNAR_MONTH_DAYS out of
range.
In `@scripts/calendar/generator.py`:
- Around line 47-53: The loop currently rebinds dts (which can be a date or
list[date]); instead normalize into a new variable (e.g., normalized_dates or
dts_list) and use that for the rest of the logic so you don't reassign the
original dts. Update the block that inspects hol_dates in the loop (the for
year, dts in sorted(hol_dates.items()) loop) to create normalized_dates = dts if
isinstance(dts, list) else [dts], then use normalized_dates when building
dates_str, wrapping it when len(normalized_dates) > 1, and when calling
YEAR_TEMPLATE.format; keep references to MONTHS and year_dates unchanged.
In `@tests/countries/test_malaysia.py`:
- Around line 252-259: Add an explicit 2011 Thaipusam exact-date assertion by
inserting the 2011 date string into the dt_1 tuple used for exact-date checks in
tests/countries/test_malaysia.py (the same place where 2012..2018 dates are
listed); update any expected-length or iteration assumptions that rely on dt_1
so the new 2011 entry is validated by the existing exact-date assertion logic.
🪄 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: 135e0c9b-1657-4357-bd10-ec24f8d54b75
📒 Files selected for processing (30)
holidays/calendars/hindu.pyholidays/countries/malaysia.pyscripts/calendar/asian_generator.pyscripts/calendar/generator.pyscripts/calendar/hebrew_generator.pyscripts/calendar/islamic_generator.pyscripts/calendar/mongolian_generator.pysnapshots/countries/KE_COMMON.jsonsnapshots/countries/MU_COMMON.jsonsnapshots/countries/MY_01.jsonsnapshots/countries/MY_02.jsonsnapshots/countries/MY_03.jsonsnapshots/countries/MY_04.jsonsnapshots/countries/MY_05.jsonsnapshots/countries/MY_06.jsonsnapshots/countries/MY_07.jsonsnapshots/countries/MY_08.jsonsnapshots/countries/MY_09.jsonsnapshots/countries/MY_10.jsonsnapshots/countries/MY_11.jsonsnapshots/countries/MY_12.jsonsnapshots/countries/MY_14.jsonsnapshots/countries/MY_15.jsonsnapshots/countries/MY_16.jsonsnapshots/countries/SG_COMMON.jsonsnapshots/countries/SR_COMMON.jsonsnapshots/countries/TT_COMMON.jsontests/countries/test_malaysia.pytests/countries/test_singapore.pytests/countries/test_trinidad_and_tobago.py
💤 Files with no reviewable changes (1)
- tests/countries/test_trinidad_and_tobago.py
Contributor
There was a problem hiding this comment.
2 issues found across 30 files
Confidence score: 3/5
- There is a concrete risk of incorrect Diwali date calculation in
scripts/calendar/asian_generator.pywhen a leap month precedes month 9, which could lead to wrong holiday outputs. - Test expectations in
tests/countries/test_malaysia.pyare out of sync with current calendar data, indicating potential regressions or stale fixtures that need adjustment. - Given a medium-severity functional bug and a test mismatch, this is a moderate-risk merge despite being localized.
- Pay close attention to
scripts/calendar/asian_generator.pyandtests/countries/test_malaysia.py- fix the month-length logic and update expected dates.
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="tests/countries/test_malaysia.py">
<violation number="1" location="tests/countries/test_malaysia.py:253">
P2: Update the Thaipusam test dates to match the current calendar data; several expected dates are off.</violation>
</file>
<file name="scripts/calendar/asian_generator.py">
<violation number="1" location="scripts/calendar/asian_generator.py:387">
P1: Bug in Diwali date calculation: `_lunar_month_days(year, holiday_month)` uses the physical bit position, but when there's a leap month before month 9, the logical month 9's length is at physical position `holiday_month + 1`. This can produce a 1-day error. The correct position for the month length is `holiday_month + (holiday_month > leap_month)`.</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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
snapshots/countries/MY_12.json (1)
1414-1913:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftConfirm the Malaysia Deepavali source switch at 2026 and move fix upstream.
The snapshot correctly reflects a fallback at 2026:
MalaysiaHinduHolidays.DIWALI_DATEScovers 2001–2026 only. From 2027 onward, the snapshot inherits the shared Hindu calendar table instead. Since Malaysia already maintains a custom Deepavali mapping for 2001–2026, regenerating snapshots with dates beyond that boundary risks inconsistency.The fix belongs in
holidays/countries/malaysia.py, not in the snapshot. Either extendDIWALI_DATESpast 2026 with Malaysia-specific values, or document why the fallback to the shared table is acceptable after 2026.🤖 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 `@snapshots/countries/MY_12.json` around lines 1414 - 1913, The snapshot shows Malaysia switching from a Malaysia-specific Deepavali table to the shared Hindu table starting 2027 because MalaysiaHinduHolidays.DIWALI_DATES only covers 2001–2026; fix this in holidays/countries/malaysia.py by either (A) extending MalaysiaHinduHolidays.DIWALI_DATES with Malaysia-specific Deepavali dates beyond 2026 to ensure consistent country-specific results, or (B) adding a clear comment/docstring and tests that assert the deliberate fallback to the shared Hindu calendar from 2027 onward so snapshot regeneration is intentional and reproducible.snapshots/countries/SG_COMMON.json (1)
14-1275: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winSnapshot looks consistent — worth a quick spot-check against an external Deepavali source.
This is a regenerated snapshot, so the diff stands or falls with the new
DIWALI_DATEStable inholidays/calendars/hindu.py. Observance pairs that I sampled (1999-11-07/08, 2040-11-04/05, 2046-10-28/29, 2050-11-13/14) all follow the Sunday→Monday rule cleanly, so the observance machinery looks fine post-refactor.Since the PR description notes Diwali/Thaipusam accuracy was the main reason for these shifts, it'd be reassuring to cross-check a handful of recent and near-future years against an authoritative source before merging.
Deepavali / Diwali date Singapore 2024 2025 2026 2027 2028 official🤖 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 `@snapshots/countries/SG_COMMON.json` around lines 14 - 1275, The snapshot changed due to the new DIWALI_DATES table in holidays/calendars/hindu.py; please verify and reconcile the regenerated SG_COMMON snapshot by cross-checking the DIWALI/Deepavali dates in DIWALI_DATES against an authoritative source for a sample of recent and near-future years (suggested years: 2024, 2025, 2026, 2027, 2028) and then either update DIWALI_DATES (in holidays/calendars/hindu.py) if you find errors or confirm and add a short test that asserts the expected Deepavali dates for those years so snapshots/snapshots/countries/SG_COMMON.json remains correct.snapshots/countries/MY_01.json (1)
4-1712: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winThaipusam table is the load-bearing change — please double-check a few years.
The Johor snapshot inherits both the new Thaipusam and Deepavali tables. PR description specifically calls out improved Thaipusam accuracy, so it's worth confirming the new entries against an external source for the years that matter most operationally (recent + near future). Sampled observance shifts (e.g., 2026-02-01 Sun → 02-02 Mon, 2050-02-06 Sun → 02-07 Mon) follow the existing observance rule consistently.
Thaipusam Malaysia public holiday dates 2024 2025 2026 2027 2028🤖 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 `@snapshots/countries/MY_01.json` around lines 4 - 1712, The Thaipusam entries in the snapshot (look for "Thaipusam" and "Thaipusam (observed)" keys) need verification for recent and near-future years (at least 2024–2028) against an authoritative source; confirm the base dates and that observance shifts follow the existing rule (Sunday → next Monday) — e.g., check 2026-02-01 vs 2026-02-02 and the 2050-02-06 → 2050-02-07 observed handling — and update the JSON entries accordingly to match the verified dates.snapshots/countries/MY_02.json (1)
1319-1320: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd explicit Thaipusam date assertions for Kedah beyond 2025.
Test coverage for Kedah's Thaipusam currently asserts the holiday name exists via
range(2022, self.end_year), but explicit date parametrization stops at 2025. The snapshot regeneration protects against unintended regressions, but explicit date cases for years like 2027, 2030, 2034, 2046, or 2050 would catch date calculation errors that snapshots alone cannot. Consider adding a few parametrized dates to cover weekday, Friday-Sunday substitute, and Saturday no-substitute scenarios in the future range.🤖 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 `@snapshots/countries/MY_02.json` around lines 1319 - 1320, The Kedah Thaipusam tests currently rely on a broad existence check using range(2022, self.end_year) but only parametrize explicit date assertions up to 2025; add explicit parametrized test cases for specific future years (e.g., 2027-01-22/01-24, 2030, 2034, 2046, 2050) to cover weekday, Fri–Sun substitute, and Sat no-substitute behaviors so date-calculation regressions are caught; update the test that asserts "Thaipusam" for Kedah (the test using range(2022, self.end_year) and the parametrized date list) to include these additional (year, expected_date_or_observed) entries and ensure they assert both the holiday name and observed variants where applicable.
♻️ Duplicate comments (1)
scripts/calendar/asian_generator.py (1)
317-330:⚠️ Potential issue | 🟠 Major | ⚡ Quick win1901 still slips into
_get_leap_month(1900)and pulls 2100 data.For
year == 1901,_get_leap_month(1901) == 15, soystays at1900andm = 12. The subsequentlunar_to_gre(1900, 12, 15)calls_get_leap_month(1900)and_lunar_month_days(1900, …), both of which indexG_LUNAR_MONTH_DAYS[-1](2100's data) — silently producing a wrong 1901 Thaipusam inhindu_dates.py. The 1952+ Malaysia snapshots don't expose it, but the generated file does. The earlier review comment on this still applies.🛡️ Suggested guard
def thaipusam_date(self, year: int) -> date: """Return the estimated Gregorian date of Thaipusam (Tamil). Defined as the date of the full moon in the Tamil month of Thai, which corresponds with the months of January or February in the Gregorian calendar. See <https://en.wikipedia.org/wiki/Thaipusam>. """ - y = year - 1 - m = 12 - if self._get_leap_month(year) < 15: - y += 1 - m = 1 - return self.lunar_to_gre(y, m, 15) + if self._get_leap_month(year) < 15: + return self.lunar_to_gre(year, 1, 15) + # Fall back to the 12th lunar month of the previous year, but stay + # within the supported lunar table range. + y = max(year - 1, self.START_YEAR) + return self.lunar_to_gre(y, 12, 15)🤖 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/asian_generator.py` around lines 317 - 330, Thaipusam_date can end up calling lunar_to_gre on year-1 (e.g., 1900 for 1901) which triggers _get_leap_month/_lunar_month_days to index out-of-range G_LUNAR_MONTH_DAYS; update thaipusam_date to guard against using a prior-year that is outside the supported range: after computing y and m, if y is less than the class's minimum supported year (or otherwise would cause _get_leap_month(y) to access bad data), set y to year (and adjust m to the appropriate Thai month) or raise a clear error; reference thaipusam_date, _get_leap_month, lunar_to_gre, _lunar_month_days and G_LUNAR_MONTH_DAYS when applying the fix.
🤖 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 193-394: DIWALI_DATES contains incorrect month-shifted entries for
years 2006, 2009, 2017, 2025 and 2028 (producing ~29–30 day gaps versus
DIWALI_INDIA_DATES); open the DIWALI_DATES mapping and for those specific years
verify the original lunisolar source (or prefer the authoritative
DIWALI_INDIA_DATES) and correct the month/day tuples so the lunisolar month
matches Diwali (i.e., adjust OCT/NOV values as required), then regenerate any
dependent snapshots/fixtures and run the holiday tests to ensure alignment.
In `@scripts/calendar/asian_generator.py`:
- Around line 372-399: The inline ternary that computes the resolved lunar day
inside generate_data increases cognitive complexity; extract it into a small
helper (either a private method on _Lunisolar, e.g. _resolve_lunar_day(year,
holiday_month, holiday_day), or a local function) that encapsulates the logic
using _lunar_month_days and _get_leap_month and returns the positive day index,
then replace the complex inline expression in generate_data with a call to that
helper (update calls inside the loop that currently use
cal._lunar_month_days(...) + holiday_day + 1 to use cal._resolve_lunar_day(...)
or the local helper).
In `@scripts/calendar/generator.py`:
- Around line 17-23: The generated *_dates.py modules can reference bare month
constants (JAN…DEC) but CLASS_TEMPLATE and HOLIDAY_DATA_TEMPLATE currently only
emit the class body, causing NameError on import; update the templates so the
output includes month constant definitions (or use numeric month literals)
before the class—e.g., add a MONTH_CONSTANTS block and prepend it into
CLASS_TEMPLATE or inline the constants into HOLIDAY_DATA_TEMPLATE so JAN..DEC
are defined when the class (like {class_name} and its {hol_name}_DATES) is
written.
---
Outside diff comments:
In `@snapshots/countries/MY_01.json`:
- Around line 4-1712: The Thaipusam entries in the snapshot (look for
"Thaipusam" and "Thaipusam (observed)" keys) need verification for recent and
near-future years (at least 2024–2028) against an authoritative source; confirm
the base dates and that observance shifts follow the existing rule (Sunday →
next Monday) — e.g., check 2026-02-01 vs 2026-02-02 and the 2050-02-06 →
2050-02-07 observed handling — and update the JSON entries accordingly to match
the verified dates.
In `@snapshots/countries/MY_02.json`:
- Around line 1319-1320: The Kedah Thaipusam tests currently rely on a broad
existence check using range(2022, self.end_year) but only parametrize explicit
date assertions up to 2025; add explicit parametrized test cases for specific
future years (e.g., 2027-01-22/01-24, 2030, 2034, 2046, 2050) to cover weekday,
Fri–Sun substitute, and Sat no-substitute behaviors so date-calculation
regressions are caught; update the test that asserts "Thaipusam" for Kedah (the
test using range(2022, self.end_year) and the parametrized date list) to include
these additional (year, expected_date_or_observed) entries and ensure they
assert both the holiday name and observed variants where applicable.
In `@snapshots/countries/MY_12.json`:
- Around line 1414-1913: The snapshot shows Malaysia switching from a
Malaysia-specific Deepavali table to the shared Hindu table starting 2027
because MalaysiaHinduHolidays.DIWALI_DATES only covers 2001–2026; fix this in
holidays/countries/malaysia.py by either (A) extending
MalaysiaHinduHolidays.DIWALI_DATES with Malaysia-specific Deepavali dates beyond
2026 to ensure consistent country-specific results, or (B) adding a clear
comment/docstring and tests that assert the deliberate fallback to the shared
Hindu calendar from 2027 onward so snapshot regeneration is intentional and
reproducible.
In `@snapshots/countries/SG_COMMON.json`:
- Around line 14-1275: The snapshot changed due to the new DIWALI_DATES table in
holidays/calendars/hindu.py; please verify and reconcile the regenerated
SG_COMMON snapshot by cross-checking the DIWALI/Deepavali dates in DIWALI_DATES
against an authoritative source for a sample of recent and near-future years
(suggested years: 2024, 2025, 2026, 2027, 2028) and then either update
DIWALI_DATES (in holidays/calendars/hindu.py) if you find errors or confirm and
add a short test that asserts the expected Deepavali dates for those years so
snapshots/snapshots/countries/SG_COMMON.json remains correct.
---
Duplicate comments:
In `@scripts/calendar/asian_generator.py`:
- Around line 317-330: Thaipusam_date can end up calling lunar_to_gre on year-1
(e.g., 1900 for 1901) which triggers _get_leap_month/_lunar_month_days to index
out-of-range G_LUNAR_MONTH_DAYS; update thaipusam_date to guard against using a
prior-year that is outside the supported range: after computing y and m, if y is
less than the class's minimum supported year (or otherwise would cause
_get_leap_month(y) to access bad data), set y to year (and adjust m to the
appropriate Thai month) or raise a clear error; reference thaipusam_date,
_get_leap_month, lunar_to_gre, _lunar_month_days and G_LUNAR_MONTH_DAYS when
applying the fix.
🪄 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: b08c01c2-385c-4ac0-985a-98f2c32a36d9
📒 Files selected for processing (24)
holidays/calendars/hindu.pyscripts/calendar/asian_generator.pyscripts/calendar/generator.pysnapshots/countries/KE_COMMON.jsonsnapshots/countries/MY_01.jsonsnapshots/countries/MY_02.jsonsnapshots/countries/MY_03.jsonsnapshots/countries/MY_04.jsonsnapshots/countries/MY_05.jsonsnapshots/countries/MY_06.jsonsnapshots/countries/MY_07.jsonsnapshots/countries/MY_08.jsonsnapshots/countries/MY_09.jsonsnapshots/countries/MY_10.jsonsnapshots/countries/MY_11.jsonsnapshots/countries/MY_12.jsonsnapshots/countries/MY_14.jsonsnapshots/countries/MY_15.jsonsnapshots/countries/MY_16.jsonsnapshots/countries/SG_COMMON.jsonsnapshots/countries/SR_COMMON.jsonsnapshots/countries/TT_COMMON.jsontests/countries/test_malaysia.pytests/countries/test_trinidad_and_tobago.py
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
holidays/calendars/hindu.py (1)
299-299:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftUnresolved:
DIWALI_DATESstill has ~30-day gaps vsDIWALI_INDIA_DATESfor 2006, 2009, 2017, 2025, and 2028.All five rows are carried forward unchanged into the new table:
Year DIWALI_DATESDIWALI_INDIA_DATESΔ 2006 NOV 20 OCT 21 +30 d 2009 NOV 16 OCT 17 +30 d 2017 NOV 17 OCT 19 +29 d 2025 NOV 19 OCT 20 +30 d 2028 NOV 15 OCT 17 +29 d External sources confirm Diwali 2028 falls on October 17, which aligns with
DIWALI_INDIA_DATESbut is a full lunar month away from theNOV 15entry inDIWALI_DATES. These are not legitimate regional-offset differences; each represents a wrong lunisolar month in the generic table.Run the following to reproduce the comparison programmatically:
#!/bin/bash python - <<'PY' from datetime import date import ast, pathlib, re src = pathlib.Path("holidays/calendars/hindu.py").read_text() JAN,FEB,MAR,APR,MAY,JUN,JUL,AUG,SEP,OCT,NOV,DEC = range(1,13) def extract_dict(name): m = re.search(rf'{name}\s*=\s*\{{(.*?)\}}', src, re.DOTALL) d = eval('{' + m.group(1) + '}') return d generic = extract_dict("DIWALI_DATES") india = extract_dict("DIWALI_INDIA_DATES") print("Year | DIWALI_DATES | DIWALI_INDIA_DATES | Delta(days)") for yr in sorted(set(generic) & set(india)): g = date(yr, *generic[yr]); i = date(yr, *india[yr]) delta = (g - i).days if abs(delta) > 2: print(f"{yr} {g} {i} {delta:+d}d ← MISMATCH") PYAlso applies to: 302-302, 310-310, 318-318, 321-321
🤖 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/calendars/hindu.py` at line 299, DIWALI_DATES contains wrong lunisolar-month entries for years 2006, 2009, 2017, 2025 and 2028 (each ~29–30 days later than the correct day in DIWALI_INDIA_DATES); update the DIWALI_DATES dictionary entries for those years to the matching dates used in DIWALI_INDIA_DATES (2006 → OCT 21, 2009 → OCT 17, 2017 → OCT 19, 2025 → OCT 20, 2028 → OCT 17) so the generic table no longer contains month-shifted values; check the dictionaries named DIWALI_DATES and DIWALI_INDIA_DATES in hindu.py and adjust the five incorrect entries (also review nearby entries at the referenced lines for similar copy-paste errors).
🤖 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 `@holidays/calendars/hindu.py`:
- Line 299: DIWALI_DATES contains wrong lunisolar-month entries for years 2006,
2009, 2017, 2025 and 2028 (each ~29–30 days later than the correct day in
DIWALI_INDIA_DATES); update the DIWALI_DATES dictionary entries for those years
to the matching dates used in DIWALI_INDIA_DATES (2006 → OCT 21, 2009 → OCT 17,
2017 → OCT 19, 2025 → OCT 20, 2028 → OCT 17) so the generic table no longer
contains month-shifted values; check the dictionaries named DIWALI_DATES and
DIWALI_INDIA_DATES in hindu.py and adjust the five incorrect entries (also
review nearby entries at the referenced lines for similar copy-paste errors).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: befd3b8d-3b0f-4200-a665-b9d96455e6fa
📒 Files selected for processing (2)
holidays/calendars/hindu.pyscripts/calendar/asian_generator.py
PPsyrius
approved these changes
May 8, 2026
PPsyrius
left a comment
Collaborator
There was a problem hiding this comment.
I can't verify Singapore's & Malaysia's Diwali/Thaipusam approximation correctness at the moment (besides 2027 ones, there's a lot of conflicting sources for 2028 onwards) - but others are LGTM
Collaborator
Author
|
Actually, I’ve left Diwali and Thaipusam just as a legacy. To achieve true accuracy, I’m hoping for Hindu calendar implementation. ✌️ |
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
3 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.
Proposed change
Refactor calendar generators (code optimization and unification).
Improved accuracy in calculating Vesak May and (especially) Thaipusam (however, their calculations based on the Chinese calendar remain approximate).
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.