Add generator for Tibetan calendar holidays#3330
Open
Abiram08 wants to merge 6 commits intovacanza:devfrom
Open
Add generator for Tibetan calendar holidays#3330Abiram08 wants to merge 6 commits intovacanza:devfrom
Abiram08 wants to merge 6 commits intovacanza:devfrom
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 a new generator script that downloads and parses Tibetan calendar data and emits a generated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip CodeRabbit can approve the review once all CodeRabbit's comments are resolved.Enable the |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a reproducible generator script intended to (re)generate Tibetan lunisolar holiday date mappings from the upstream “bhutanese-calendar” plugin data, and updates holidays/calendars/tibetan.py with newly generated output.
Changes:
- Added
scripts/calendar/tibetan_generator.pyto download/parse upstream Tibetan calendar data and emitholidays/calendars/tibetan.py. - Regenerated
holidays/calendars/tibetan.pywith new date dictionaries and updated Gregorian month imports/constants formatting.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| scripts/calendar/tibetan_generator.py | New generator for producing Tibetan holiday date dictionaries from upstream data. |
| holidays/calendars/tibetan.py | Regenerated Tibetan holiday date tables (and import formatting) based on the new generator output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
holidays/calendars/tibetan.py
Outdated
| 2097: (JUN, 19), | ||
| 2098: (JUL, 8), | ||
| 2099: (JUN, 27), | ||
| BIRTH_OF_GURU_RINPOCHE_DATES_DATES = { |
holidays/calendars/tibetan.py
Outdated
Comment on lines
+2008
to
+2013
| 2020: (SEP, 22), | ||
| 2021: (SEP, 22), | ||
| 2022: (SEP, 22), | ||
| 2023: (SEP, 22), | ||
| 2024: (SEP, 22), | ||
| 2025: (SEP, 22), |
![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: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/calendar/tibetan_generator.py`:
- Around line 204-214: The exception handler around urlretrieve(DATA_URL,
data_file) should preserve the original exception chain; update the except block
that catches Exception as e to re-raise the RuntimeError with the original
exception as its cause (i.e., use "raise RuntimeError('No data file available
and download failed!') from e") so callers can see the original error
originating from urlretrieve / DATA_URL / data_file while keeping the existing
cached-file fallback and the return of data_file intact.
- Around line 338-340: Before calling output_path.write_text(...) ensure the
target directory exists and is valid: check Path(__file__).parent.parent.parent
/ "holidays" / "calendars" (the parent of output_path and the directory that
will contain OUT_FILE_NAME) and either create it (e.g., mkdir(parents=True,
exist_ok=True)) or raise a clear exception with the resolved path; update the
code around the output_path and write_text calls to perform this
existence/creation check and fail with a helpful error message if creation is
not possible.
- Around line 274-277: The nested condition can be flattened: replace the inner
if within the block that checks g_month == greg_month so a single if checks both
g_month == greg_month and greg_day in greg_days (and the existing greg_year not
in years_seen) in one expression; update the block that writes to dates_dict
(using MONTH_NAMES[g_month], greg_day) and adds greg_year to years_seen to
remain the same, just under the single combined if that references g_month,
greg_month, greg_day, greg_days, greg_year, years_seen, dates_dict, and
MONTH_NAMES.
- Around line 222-238: The loop currently reassigns the loop variable line which
obscures intent and triggers PLW2901; change the loop to read a distinct name
(e.g., raw_line) in the for-loop and use a separate variable for the processed
string (e.g., stripped or eval_text) before calling ast.literal_eval and
appending day_tuple; update references in the try block (the rstrip,
strip/checks, and literal_eval) to use these new names so the original loop
variable is not overwritten.
- Around line 244-261: The function extract_holiday_dates blindly indexes into
each day_data tuple (indices 2,3,6,7,8) which can raise IndexError if the input
format changes; add a defensive check—either validate len(day_data) >= 9 before
unpacking or wrap the unpacking in a try/except that catches
IndexError/TypeError and raises a clearer ValueError with context including the
offending day_data and its index (or log and continue), then proceed to use
tib_day, tib_month, greg_day, greg_month, greg_year and MONTH_NAMES as before to
populate dates_dict.
- Around line 129-132: HOLIDAY_DATA_TEMPLATE currently appends a literal
"_DATES" to {hol_name}, causing double-suffix names like
BIRTH_OF_GURU_RINPOCHE_DATES_DATES; change the template so it uses {hol_name}
(no added suffix) — i.e. replace "{hol_name}_DATES" with "{hol_name}" in
HOLIDAY_DATA_TEMPLATE — so generated attributes match what _get_holiday expects
(it looks up f"{holiday}_DATES") and work with callers that already pass
f"{hol_name}_DATES".
🪄 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: 2718dbe6-09c2-4793-9903-e38a0133c159
📒 Files selected for processing (2)
holidays/calendars/tibetan.pyscripts/calendar/tibetan_generator.py
Abiram08
force-pushed
the
feature/tibetan-calendar-generator-clean
branch
from
d3ac2c6 to
3584f90
Compare
Contributor
Author
|
Fixed those bugs. Let me know if there are any changes! |
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 `@scripts/calendar/tibetan_generator.py`:
- Around line 312-314: The generator currently prints "ERROR: No data parsed!"
and returns when all_days is empty (the branch checking the all_days variable in
the top-level generation function), which leaves the process exit code as 0;
change this to raise an exception (for example RuntimeError or a custom
ParseError) with a clear message so the script fails fast when no rows are
parsed (update the branch that currently prints and returns to raise instead),
ensuring callers and CI detect the failure instead of silently keeping a stale
tibetan.py.
- Around line 182-204: The download_data function currently writes directly to
DATA_FILENAME (data_file) so a failed refresh can leave a truncated cache;
change download logic to write to a temporary path (e.g.,
data_file.with_suffix('.tmp') or similar) when calling urlretrieve(DATA_URL,
...), only replace/rename the temp file to data_file on successful completion,
and ensure partial temp files are cleaned up on failure; reference
download_data, DATA_FILENAME/DATA_URL, urlretrieve and data_file when applying
this fix.
- Around line 268-282: The helper extract_fixed_holiday() is being misused for
BLESSED_RAINY_DAY because it simply picks the first matching greg_day per year
(so SEP 22 wins), but BLESSED_RAINY_DAY is equinox-based and needs a
year-specific resolver; stop using extract_fixed_holiday() for BLESSED_RAINY_DAY
and implement a dedicated per-year resolver (e.g.,
compute_blessed_rainy_day_for_year or resolve_equinox_based_holiday) that
inspects all_days for the specific year and applies the correct equinox/year
rule (or an astronomical/lookup algorithm), validates the candidate with
date(...), and returns the correct (MONTH_NAMES[...], day) for that year so
BLESSED_RAINY_DAY_DATES is built from year-by-year resolution rather than
first-match logic; update call sites to use the new resolver and keep
extract_fixed_holiday() only for truly fixed Gregorian dates.
- Around line 34-35: DATA_URL currently points to the branch ref
"refs/heads/master" which makes generated output non-reproducible; instead pin
the upstream snapshot by replacing DATA_URL with a raw URL that references a
specific commit SHA or tag (not a branch) so the script always fetches the same
data; update the constant DATA_URL in tibetan_generator.py to use the raw GitHub
URL with the chosen commit SHA (keep DATA_FILENAME and any downstream usages the
same) and verify fetching still works.
🪄 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: c4bab4a5-3ba6-480f-8ffc-3073c9ecd99a
📒 Files selected for processing (2)
holidays/calendars/tibetan.pyscripts/calendar/tibetan_generator.py
Comment on lines
+34
to
+35
| DATA_URL = "https://github.com/wp-plugins/bhutanese-calendar/raw/refs/heads/master/data/data.txt" | ||
| DATA_FILENAME = "tibetan_data.txt" |
Contributor
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
import urllib.request
with urllib.request.urlopen(
"https://api.github.com/repos/wp-plugins/bhutanese-calendar/branches/master"
) as resp:
data = json.load(resp)
print(data["commit"]["sha"])
PYRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 1
stderr:
Traceback (most recent call last):
File "/usr/lib/python3.11/urllib/request.py", line 1348, in do_open
h.request(req.get_method(), req.selector, req.data, headers,
File "/usr/lib/python3.11/http/client.py", line 1282, in request
self._send_request(method, url, body, headers, encode_chunked)
File "/usr/lib/python3.11/http/client.py", line 1328, in _send_request
self.endheaders(body, encode_chunked=encode_chunked)
File "/usr/lib/python3.11/http/client.py", line 1277, in endheaders
self._send_output(message_body, encode_chunked=encode_chunked)
File "/usr/lib/python3.11/http/client.py", line 1037, in _send_output
self.send(msg)
File "/usr/lib/python3.11/http/client.py", line 975, in send
self.connect()
File "/usr/lib/python3.11/http/client.py", line 1454, in connect
self.sock = self._context.wrap_socket(self.sock,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/ssl.py", line 517, in wrap_socket
return self.sslsocket_class._create(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/ssl.py", line 1108, in _create
self.do_handshake()
File "/usr/lib/python3.11/ssl.py", line 1379, in do_handshake
self._sslobj.do_handshake()
ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:992)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 4, in <module>
File "/usr/lib/python3.11/urllib/request.py", line 216, in urlopen
return opener.open(url, data, timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 519, in open
response = self._open(req, data)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 536, in _open
result = self._call_chain(self.handle_open, protocol, protocol +
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 496, in _call_chain
result = func(*args)
^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 1391, in https_open
return self.do_open(http.client.HTTPSConnection, req,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/urllib/request.py", line 1351, in do_open
raise URLError(err)
urllib.error.URLError: <urlopen error [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:992)>
Script executed:
# Check if the file exists and inspect lines 34-35
fd tibetan_generator.py --type fRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
scripts/calendar/tibetan_generator.py
Script executed:
# Check the file around lines 34-35 and get context
head -50 scripts/calendar/tibetan_generator.py | cat -nRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
1 #!/usr/bin/env python3
2
3 # holidays
4 # --------
5 # A fast, efficient Python library for generating country, province and state
6 # specific sets of holidays on the fly. It aims to make determining whether a
7 # specific date is a holiday as fast and flexible as possible.
8 #
9 # Authors: Vacanza Team and individual contributors (see CONTRIBUTORS file)
10 # dr-prodigy <dr.prodigy.github@gmail.com> (c) 2017-2023
11 # ryanss <ryanssdev@icloud.com> (c) 2014-2017
12 # Website: https://github.com/vacanza/holidays
13 # License: MIT (see LICENSE file)
14
15 """
16 Generate Tibetan calendar holidays from WordPress plugin data.
17
18 Downloads calendar data from bhutanese-calendar plugin, extracts Gregorian
19 dates for each Tibetan holiday, and generates the _TibetanLunisolar class.
20
21 Usage:
22 python tibetan_generator.py # use cached data
23 python tibetan_generator.py --download # force fresh download
24 """
25
26 # ruff: noqa: T201, FBT001, FBT002, S310
27
28 import ast
29 import sys
30 from datetime import date
31 from pathlib import Path
32 from urllib.request import urlretrieve
33
34 DATA_URL = "https://github.com/wp-plugins/bhutanese-calendar/raw/refs/heads/master/data/data.txt"
35 DATA_FILENAME = "tibetan_data.txt"
36
37 # Tibetan calendar dates for each holiday (day, month in Tibetan calendar)
38 HOLIDAY_DATES = {
39 "BIRTH_OF_GURU_RINPOCHE": (10, 5),
40 "BUDDHA_PARINIRVANA": (15, 4),
41 "BUDDHA_FIRST_SERMON": (4, 6),
42 "DESCENDING_DAY_OF_LORD_BUDDHA": (22, 9),
43 "THIMPHU_DRUBCHEN": (16, 8),
44 "THIMPHU_TSHECHU": (20, 8),
45 "DEATH_OF_ZHABDRUNG": (10, 3),
46 "DAY_OF_OFFERING": (30, 11),
47 "LOSAR": (1, 1),
48 }
49
50 # These follow Gregorian calendar, not Tibetan
Script executed:
# Get more context - check download_data function and understand the flow
sed -n '180,210p' scripts/calendar/tibetan_generator.py | cat -nRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
1
2
3 def download_data(force: bool = False) -> Path:
4 """Download calendar data or use cached version."""
5 data_file = Path(__file__).parent / DATA_FILENAME
6
7 if data_file.exists() and not force:
8 print(f"Using existing data file: {data_file}")
9 return data_file
10
11 print(f"Downloading data from: {DATA_URL}")
12 print(f"Saving to: {data_file}")
13
14 try:
15 urlretrieve(DATA_URL, data_file)
16 print(f"✅ Download complete! ({data_file.stat().st_size} bytes)")
17 except Exception as e:
18 print(f"❌ Download failed: {e}")
19 if data_file.exists():
20 print(f"Using existing cached file: {data_file}")
21 else:
22 msg = "No data file available and download failed!"
23 raise RuntimeError(msg) from e
24
25 return data_file
26
27
28 def parse_data_file(filename: Path) -> list:
29 """Parse data file into list of day tuples.
30
31 Each line is a tuple:
Script executed:
# Check if there are other calendar generators and how they handle URLs
find scripts/calendar -name "*_generator.py" -type f | head -10Repository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
scripts/calendar/asian_generator.py
scripts/calendar/hebrew_generator.py
scripts/calendar/islamic_generator.py
scripts/calendar/mongolian_generator.py
scripts/calendar/tibetan_generator.py
Script executed:
# Check how other generators handle DATA_URL - look for patterns
for file in scripts/calendar/{asian,hebrew,islamic,mongolian}_generator.py; do
echo "=== $file ==="
grep -n "DATA_URL\|refs/heads\|commit\|SHA" "$file" || echo "No URL found"
doneRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
=== scripts/calendar/asian_generator.py ===
No URL found
=== scripts/calendar/hebrew_generator.py ===
38: (3, 6, "SHAVUOT"),
39: (5, 9, "TISHA_BAV"),
40: (7, 1, "ROSH_HASHANAH"),
=== scripts/calendar/islamic_generator.py ===
No URL found
=== scripts/calendar/mongolian_generator.py ===
No URL found
Script executed:
# Check if other generators actually fetch from URLs
for file in scripts/calendar/{asian,hebrew,islamic,mongolian}_generator.py; do
echo "=== $(basename $file) ==="
head -50 "$file" | grep -E "http|url|URL"
doneRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
=== asian_generator.py ===
# Website: https://github.com/vacanza/holidays
<https://en.wikipedia.org/wiki/Chinese_New_Year#Dates_in_Chinese_\
http://putidea.blogspot.com/2010/10/python.html
https://github.com/BohanZhang/python-MagicLamp/blob/master/bin/utils/lunar_calendar.py
=== hebrew_generator.py ===
# Website: https://github.com/vacanza/holidays
=== islamic_generator.py ===
# Website: https://github.com/vacanza/holidays
=== mongolian_generator.py ===
# Website: https://github.com/vacanza/holidays
See the Wikipedia article: https://en.wikipedia.org/wiki/Mongolian_calendar
- https://www.math.mcgill.ca/gantumur/cal/year.html
Script executed:
# Check the full context of tibetan_generator.py - especially around the main function
tail -50 scripts/calendar/tibetan_generator.py | cat -nRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
1 print(f"Data source: {DATA_URL}")
2 print("=" * 70)
3
4 data_file = download_data(force=force_download)
5 all_days = parse_data_file(data_file)
6
7 if not all_days:
8 print("ERROR: No data parsed!")
9 return
10
11 holiday_data = []
12
13 # Extract Tibetan calendar holidays
14 for hol_name, (tib_day, tib_month) in sorted(HOLIDAY_DATES.items()):
15 print(f"Extracting {hol_name} (Tibetan {tib_day}/{tib_month})...")
16 dates_dict = extract_holiday_dates(all_days, tib_day, tib_month)
17 print(f" Found {len(dates_dict)} occurrences")
18 # Template adds _DATES suffix, so pass base name only
19 holiday_data.append(generate_holiday_dict_code(hol_name, dates_dict))
20
21 # Extract Gregorian-based holidays
22 for hol_name, (greg_month, greg_days) in sorted(FIXED_HOLIDAYS.items()):
23 print(f"Extracting {hol_name} (Gregorian month {greg_month})...")
24 dates_dict = extract_fixed_holiday(all_days, greg_month, greg_days)
25 print(f" Found {len(dates_dict)} occurrences")
26 holiday_data.append(generate_holiday_dict_code(hol_name, dates_dict))
27
28 # Assemble complete file
29 holiday_data_str = "\n".join(holiday_data)
30 class_str = (
31 HEADER_TEMPLATE.format(class_name=CLASS_NAME) + "\n" + holiday_data_str + FOOTER_TEMPLATE
32 )
33
34 # Write output
35 output_path = Path(__file__).parent.parent.parent / "holidays" / "calendars" / OUT_FILE_NAME
36
37 if not output_path.parent.exists():
38 msg = f"Output directory does not exist: {output_path.parent}"
39 raise RuntimeError(msg)
40
41 output_path.write_text(class_str, encoding="UTF-8")
42
43 print(f"\n✅ Generated: {output_path}")
44 print(f"File size: {len(class_str)} bytes")
45 print("\nDone!")
46
47
48 if __name__ == "__main__":
49 force_download = "--download" in sys.argv
50 generate_data(force_download=force_download)
Script executed:
# Check the main function signature and error handling
sed -n '300,330p' scripts/calendar/tibetan_generator.py | cat -nRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
1
2 def generate_data(force_download: bool = False) -> None:
3 """Generate tibetan.py from calendar data."""
4 print("=" * 70)
5 print("Tibetan Calendar Holiday Generator")
6 print("=" * 70)
7 print(f"Data source: {DATA_URL}")
8 print("=" * 70)
9
10 data_file = download_data(force=force_download)
11 all_days = parse_data_file(data_file)
12
13 if not all_days:
14 print("ERROR: No data parsed!")
15 return
16
17 holiday_data = []
18
19 # Extract Tibetan calendar holidays
20 for hol_name, (tib_day, tib_month) in sorted(HOLIDAY_DATES.items()):
21 print(f"Extracting {hol_name} (Tibetan {tib_day}/{tib_month})...")
22 dates_dict = extract_holiday_dates(all_days, tib_day, tib_month)
23 print(f" Found {len(dates_dict)} occurrences")
24 # Template adds _DATES suffix, so pass base name only
25 holiday_data.append(generate_holiday_dict_code(hol_name, dates_dict))
26
27 # Extract Gregorian-based holidays
28 for hol_name, (greg_month, greg_days) in sorted(FIXED_HOLIDAYS.items()):
29 print(f"Extracting {hol_name} (Gregorian month {greg_month})...")
30 dates_dict = extract_fixed_holiday(all_days, greg_month, greg_days)
31 print(f" Found {len(dates_dict)} occurrences")
Script executed:
# Verify the exit behavior - check if there's any sys.exit() call in main
sed -n '320,350p' scripts/calendar/tibetan_generator.py | cat -nRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
1 print(f"Extracting {hol_name} (Tibetan {tib_day}/{tib_month})...")
2 dates_dict = extract_holiday_dates(all_days, tib_day, tib_month)
3 print(f" Found {len(dates_dict)} occurrences")
4 # Template adds _DATES suffix, so pass base name only
5 holiday_data.append(generate_holiday_dict_code(hol_name, dates_dict))
6
7 # Extract Gregorian-based holidays
8 for hol_name, (greg_month, greg_days) in sorted(FIXED_HOLIDAYS.items()):
9 print(f"Extracting {hol_name} (Gregorian month {greg_month})...")
10 dates_dict = extract_fixed_holiday(all_days, greg_month, greg_days)
11 print(f" Found {len(dates_dict)} occurrences")
12 holiday_data.append(generate_holiday_dict_code(hol_name, dates_dict))
13
14 # Assemble complete file
15 holiday_data_str = "\n".join(holiday_data)
16 class_str = (
17 HEADER_TEMPLATE.format(class_name=CLASS_NAME) + "\n" + holiday_data_str + FOOTER_TEMPLATE
18 )
19
20 # Write output
21 output_path = Path(__file__).parent.parent.parent / "holidays" / "calendars" / OUT_FILE_NAME
22
23 if not output_path.parent.exists():
24 msg = f"Output directory does not exist: {output_path.parent}"
25 raise RuntimeError(msg)
26
27 output_path.write_text(class_str, encoding="UTF-8")
28
29 print(f"\n✅ Generated: {output_path}")
30 print(f"File size: {len(class_str)} bytes")
31 print("\nDone!")
Script executed:
# Check the actual __main__ block
sed -n '348,360p' scripts/calendar/tibetan_generator.py | cat -nRepository: vacanza/holidays
Repository: vacanza/holidays
Exit code: 0
stdout:
1 print(f"\n✅ Generated: {output_path}")
2 print(f"File size: {len(class_str)} bytes")
3 print("\nDone!")
4
5
6 if __name__ == "__main__":
7 force_download = "--download" in sys.argv
8 generate_data(force_download=force_download)
Pin the upstream data snapshot.
DATA_URL tracks refs/heads/master, so rerunning the generator later can rewrite historical tables from a different upstream state. That misses the reproducibility goal of this script.
♻️ Proposed fix
-DATA_URL = "https://github.com/wp-plugins/bhutanese-calendar/raw/refs/heads/master/data/data.txt"
+UPSTREAM_DATA_SHA = "<pin-the-reviewed-upstream-commit>"
+DATA_URL = (
+ "https://raw.githubusercontent.com/"
+ f"wp-plugins/bhutanese-calendar/{UPSTREAM_DATA_SHA}/data/data.txt"
+)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/calendar/tibetan_generator.py` around lines 34 - 35, DATA_URL
currently points to the branch ref "refs/heads/master" which makes generated
output non-reproducible; instead pin the upstream snapshot by replacing DATA_URL
with a raw URL that references a specific commit SHA or tag (not a branch) so
the script always fetches the same data; update the constant DATA_URL in
tibetan_generator.py to use the raw GitHub URL with the chosen commit SHA (keep
DATA_FILENAME and any downstream usages the same) and verify fetching still
works.
Collaborator
|
General remark: the code should be simple; take a look at other generators. print(f"❌ Download failed: {e}")A very clear example of AI code. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/calendar/tibetan_generator.py (1)
180-188:⚠️ Potential issue | 🟠 MajorThis still hard-codes September 23.
daysis built from Gregorian dates 22/23/24, so a complete year will always contain23andchosencollapses to23. That means the generatedBLESSED_RAINY_DAY_DATEStable is not actually resolving an equinox-based holiday year by year. Either source a per-year lookup or make the fixed approximation explicit instead of deriving it fromall_days.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/calendar/tibetan_generator.py` around lines 180 - 188, The code incorrectly assumes Sept 23 will always be present in rainy because days is prefiltered to (22,23,24); instead choose the day closest to the autumnal equinox per year (or explicitly document a fixed approximation). Replace the current chosen selection (chosen = 23 if 23 in days else ...) with logic that picks the day from rainy[year] minimizing abs(day - 23) (tie-breaker: pick the earlier date), so BLESSED_RAINY_DAY for each year is the closest of the available Gregorian dates; alternatively, if you intend a fixed approximation, stop deriving from all_days and set chosen = 23 unconditionally and add a comment explaining it. Ensure changes reference rainy, days, chosen, BLESSED_RAINY_DAY, and dates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/calendar/tibetan_generator.py`:
- Around line 177-178: The code uses _date(...).strftime("%b").upper() to build
month_str which is locale-dependent; replace this with an explicit static
month-abbreviation mapping (e.g. MONTH_ABBR list or dict) and set month_str =
MONTH_ABBR[g_month - 1] before constructing the dates.setdefault(hol_name,
{})[g_year] = f"{month_str}, {g_day}"; update or add the MONTH_ABBR constant
near the top of tibetan_generator.py and reference it instead of strftime to
ensure consistent English abbreviations.
---
Duplicate comments:
In `@scripts/calendar/tibetan_generator.py`:
- Around line 180-188: The code incorrectly assumes Sept 23 will always be
present in rainy because days is prefiltered to (22,23,24); instead choose the
day closest to the autumnal equinox per year (or explicitly document a fixed
approximation). Replace the current chosen selection (chosen = 23 if 23 in days
else ...) with logic that picks the day from rainy[year] minimizing abs(day -
23) (tie-breaker: pick the earlier date), so BLESSED_RAINY_DAY for each year is
the closest of the available Gregorian dates; alternatively, if you intend a
fixed approximation, stop deriving from all_days and set chosen = 23
unconditionally and add a comment explaining it. Ensure changes reference rainy,
days, chosen, BLESSED_RAINY_DAY, and dates.
🪄 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: 0cf488dc-4d2d-4077-97c3-daf501987eb9
📒 Files selected for processing (2)
holidays/calendars/tibetan.pyscripts/calendar/tibetan_generator.py
Comment on lines
+177
to
+178
| month_str = _date(g_year, g_month, 1).strftime("%b").upper() | ||
| dates.setdefault(hol_name, {})[g_year] = f"{month_str}, {g_day}" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Python datetime.strftime("%b") depend on the active locale?
💡 Result:
Yes. In Python, datetime.strftime() ultimately delegates to the platform C library’s strftime(), and directives like "%b" are defined as “month as locale’s abbreviated name”, so the output depends on the active LC_TIME locale (e.g., "Jan" vs a non-English abbreviation). [1], [2]
You can change what "%b" produces by changing the process locale via locale.setlocale(locale.LC_TIME, ...) (which affects locale-sensitive formatting). [3]
Sources:
[1] Python datetime docs: "%b" is locale’s abbreviated month name; notes Python calls platform C strftime() (platform/locale dependent). (docs.python.org)
[2] POSIX strftime spec: "%b" replaced by the locale’s abbreviated month name. (man7.org)
[3] Python locale module docs (locale affects time.strftime()-style formatting). (docs.python.org)
Citations:
- 1: https://docs.python.org/3.14/library/datetime.html?utm_source=openai
- 2: https://man7.org/linux/man-pages/man3/strftime.3p.html?utm_source=openai
- 3: https://docs.python.org/3.11/library/locale.html?utm_source=openai
🏁 Script executed:
# First, check if the file exists and get its structure
find . -name "tibetan_generator.py" -type fRepository: vacanza/holidays
Length of output: 99
🏁 Script executed:
# Get the file size to determine how to read it
wc -l scripts/calendar/tibetan_generator.pyRepository: vacanza/holidays
Length of output: 101
🏁 Script executed:
# Examine the relevant section around lines 177-178
sed -n '1,30p' scripts/calendar/tibetan_generator.pyRepository: vacanza/holidays
Length of output: 1150
🏁 Script executed:
# Get context around the flagged lines
sed -n '170,185p' scripts/calendar/tibetan_generator.pyRepository: vacanza/holidays
Length of output: 734
🏁 Script executed:
# Search for uses of strftime in the file to understand the scope
rg -n "strftime" scripts/calendar/tibetan_generator.pyRepository: vacanza/holidays
Length of output: 140
🏁 Script executed:
# Check if MONTH_NAMES or similar month mappings already exist
rg -n "MONTH" scripts/calendar/tibetan_generator.pyRepository: vacanza/holidays
Length of output: 42
🏁 Script executed:
# Check all uses of _date to see if removing the import would break anything
rg -n "_date" scripts/calendar/tibetan_generator.pyRepository: vacanza/holidays
Length of output: 1424
Replace locale-dependent strftime() with explicit month mapping.
strftime("%b") returns the locale's abbreviated month name. Running this under non-English locales emits tokens like MÄR or MAI instead of the expected "MAR" or "MAI" constants, breaking the generated output. Use a static month-number mapping instead.
Suggested fix
import ast
-from datetime import date as _date
from pathlib import Path
from urllib.request import urlretrieve
+MONTH_NAMES = {
+ 1: "JAN",
+ 2: "FEB",
+ 3: "MAR",
+ 4: "APR",
+ 5: "MAY",
+ 6: "JUN",
+ 7: "JUL",
+ 8: "AUG",
+ 9: "SEP",
+ 10: "OCT",
+ 11: "NOV",
+ 12: "DEC",
+}
+
...
- month_str = _date(g_year, g_month, 1).strftime("%b").upper()
+ month_str = MONTH_NAMES[g_month]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/calendar/tibetan_generator.py` around lines 177 - 178, The code uses
_date(...).strftime("%b").upper() to build month_str which is locale-dependent;
replace this with an explicit static month-abbreviation mapping (e.g. MONTH_ABBR
list or dict) and set month_str = MONTH_ABBR[g_month - 1] before constructing
the dates.setdefault(hol_name, {})[g_year] = f"{month_str}, {g_day}"; update or
add the MONTH_ABBR constant near the top of tibetan_generator.py and reference
it instead of strftime to ensure consistent English abbreviations.
✅ Addressed in commit 04a1824
Collaborator
There was a problem hiding this comment.
@coderabbitai, this approach is used in all other generators.
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check strftime usage across all generator scripts
rg -n "strftime" scripts/calendar/ --type py -C2
Length of output: 1804
@KJhellico, you're right. strftime('%b').upper() is the established pattern used consistently across mongolian_generator.py, islamic_generator.py, hebrew_generator.py, and asian_generator.py. My comment was incorrect — apologies for the noise.
✏️ Learnings added
Learnt from: KJhellico
Repo: vacanza/holidays PR: 3330
File: scripts/calendar/tibetan_generator.py:0-0
Timestamp: 2026-03-16T16:44:16.288Z
Learning: In the vacanza/holidays project, generator scripts (scripts/calendar/) consistently use `dt.strftime('%b').upper()` to produce uppercase month abbreviations (e.g., "JAN", "FEB") for generating date strings. This is the established project-wide convention; do not flag it as locale-dependent or suggest replacing it with a static mapping.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-19T16:54:58.657Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2416
File: tests/countries/test_turkmenistan.py:52-64
Timestamp: 2025-04-05T04:47:27.213Z
Learning: For holiday tests in the vacanza/holidays project, structure tests by individual holidays rather than by years. Each test method should focus on a specific holiday and test it across multiple years (from start_year through 2050) using helper methods like `assertHolidayName`. For fixed holidays, use generators like `(f"{year}-01-01" for year in range(1991, 2051))`. For movable holidays, specify individual dates for specific years followed by a range check.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:15-16
Timestamp: 2025-08-21T05:56:33.276Z
Learning: In the holidays library, when importing Gregorian month constants from holidays.calendars.gregorian, only import the months that are actually used in the date data. For example, if Islamic holiday dates only reference JUN, JUL, SEP, OCT, then only import those specific constants rather than importing additional unused months.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2643
File: holidays/countries/mauritius.py:144-169
Timestamp: 2025-06-19T02:34:18.382Z
Learning: Custom holiday classes that extend _CustomHinduHolidays, _CustomIslamicHolidays, _CustomBuddhistHolidays, etc. in the holidays library do not use docstrings. They follow a pattern of using only inline comments above date dictionaries, as seen in Malaysia, Singapore, UAE, and other country implementations.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: tests/countries/test_iran.py:28-28
Timestamp: 2025-09-14T16:05:55.205Z
Learning: In tests/countries/test_iran.py, using years=(self.start_year - 1, 2102) in the no-holiday test is intentional because Iran uses the Persian Calendar which has specific supported year range constraints, and 2102 represents the upper limit of the Persian Calendar's supported range, not just an arbitrary far-future date.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2706
File: holidays/countries/cayman_islands.py:80-97
Timestamp: 2025-07-10T03:36:16.461Z
Learning: In the holidays library, date dictionaries that map years to specific dates (like queens_birthday_dates, spring_bank_dates, thanksgiving_day_dates, etc.) are typically defined within the _populate_public_holidays method rather than as module-level constants. This is the established library-wide pattern seen across multiple country implementations including United Kingdom, United States, Sri Lanka, and others.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2614
File: holidays/countries/guyana.py:146-179
Timestamp: 2025-06-14T20:12:37.212Z
Learning: The `_CustomHinduHolidays` mechanism works through `_CustomCalendarType` metaclass which renames public attributes (like `DIWALI_INDIA_DATES`) with a postfix, allowing `_HinduLunisolar::_get_holiday` to find and use custom holiday dates. When `_add_diwali_india()` is called, it uses the custom dates if available rather than calculated dates.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2614
File: holidays/countries/guyana.py:146-179
Timestamp: 2025-06-14T20:12:37.212Z
Learning: The `_CustomHinduHolidays` mechanism works through `_CustomCalendarType` metaclass which renames public attributes (like `DIWALI_INDIA_DATES`) with a postfix, allowing `_HinduLunisolar::_get_holiday` to find and use custom holiday dates. When `_add_diwali_india()` is called, it uses the custom dates if available rather than calculated dates.
Learnt from: Wasif-Shahzad
Repo: vacanza/holidays PR: 2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants like FEB, TUE, MAR, and SUN internally through parent class implementations even when these constants don't appear directly in the file.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2601
File: holidays/countries/mongolia.py:140-146
Timestamp: 2025-06-15T11:50:44.366Z
Learning: The holidays library doesn't currently have helper functions like `_add_holiday_span` for adding consecutive multi-day holidays. Countries like Mongolia, Thailand, and Vietnam implement multi-day holiday spans using inline loops with `_timedelta(dt, delta)` pattern.
Learnt from: Wasif-Shahzad
Repo: vacanza/holidays PR: 2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2860
File: holidays/groups/eastern.py:36-36
Timestamp: 2025-08-28T02:34:11.657Z
Learning: The default estimated_label change from "%s (estimated)" to "%s" in holidays/groups/eastern.py is intentional to remove English-only fallback text from localized holiday names. Countries that need estimated labels should define them properly with localization support rather than relying on the English fallback.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2860
File: holidays/countries/burkina_faso.py:27-30
Timestamp: 2025-08-26T21:24:41.827Z
Learning: Countries in the holidays library that don't have localization support yet should use plain English strings for labels (e.g., `estimated_label = "%s (estimated)"`), while only countries with existing .po translation files should use `tr()` wrapping. Check for the presence of .po files in holidays/locale to determine if a country has localization support.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2794
File: holidays/groups/christian.py:328-343
Timestamp: 2025-08-11T10:14:28.517Z
Learning: For Ethiopian holidays in the `holidays/groups/christian.py` file, docstring wording should maintain source-accurate phrasing (e.g., "in coincidence of" for Ethiopian New Year/Enkutatash), even when it might read awkwardly in English, to ensure consistency with official Ethiopian documentation.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2960
File: holidays/locale/ca/LC_MESSAGES/ES.po:97-100
Timestamp: 2025-09-26T13:44:12.652Z
Learning: When fixing localization issues in the holidays project, corrections should be made in both the Python source code comments and the en_US .po file msgstr entries, then all other language .po files should be regenerated to pick up the corrected en_US comments.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
Learning: In the holidays library, countries with localization support consistently use tr() wrappers around holiday names when calling _add_* methods (e.g., self._add_new_years_day(tr("Holiday Name"))). This is the established pattern across United States, Thailand, and other l10n-enabled countries, contrary to any suggestion that translation is handled internally by _add_* methods.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
Learning: In the holidays library, countries with localization support DO use tr() wrappers around holiday names when calling _add_* methods. This is the correct pattern for l10n-enabled country implementations, contrary to previous learning about translation being handled internally by _add_* methods.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2881
File: holidays/locale/fr/LC_MESSAGES/DZ.po:37-41
Timestamp: 2025-09-03T17:10:35.672Z
Learning: In Algerian French localization (holidays/locale/fr/LC_MESSAGES/DZ.po), capitalization should follow Algerian source-accurate conventions rather than standard French orthographic rules. For example, "Jour de l'An Grégorien" with capital "G" is correct for Algerian usage, even though standard French would typically use lowercase "grégorien".
Learnt from: ankushhKapoor
Repo: vacanza/holidays PR: 2601
File: holidays/calendars/mongolian.py:1-1
Timestamp: 2025-06-10T12:43:10.577Z
Learning: The holidays project's calendar files (in holidays/calendars/) follow a consistent pattern of NOT having module-level docstrings. They only use class-level docstrings for their main classes. When reviewing calendar files, maintain this consistency and do not suggest adding module docstrings.
Learnt from: PPsyrius
Repo: vacanza/holidays PR: 2794
File: holidays/calendars/ethiopian.py:13-13
Timestamp: 2025-08-11T13:48:45.953Z
Learning: The holidays library does not use `__all__` declarations in calendar modules (holidays/calendars/). Calendar files follow a standard pattern of defining constants and functions directly without explicit exports, similar to the convention used in country modules.
Learnt from: KJhellico
Repo: vacanza/holidays PR: 2653
File: holidays/locale/th/LC_MESSAGES/TW.po:17-21
Timestamp: 2025-06-21T18:06:50.027Z
Learning: KJhellico's username includes a tilde character (~) as part of their nickname (appears as "~Jhellico" in Last-Translator headers), which is intentional formatting and not an error.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/calendar/tibetan_generator.py (1)
176-200: 🧹 Nitpick | 🔵 TrivialConsider defensive tuple validation.
These functions access indices 2, 3, 6, 7, 8 directly. If the upstream data format ever changes, an
IndexErrorwould occur with a non-obvious message.A simple guard could help:
if len(d) < 9: continueGiven the data source is frozen, this is optional but would make debugging easier if data corruption occurs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/calendar/tibetan_generator.py` around lines 176 - 200, Add defensive tuple-length guards to the three extraction functions to avoid IndexError if upstream day tuples are short: in _extract_tibetan_holidays, _extract_blessed_rainy_day and _extract_winter_solstice, skip any day tuple d where len(d) < 9 (or the number of indices you access) before reading d[2], d[3], d[6], d[7], d[8]; keep the rest of the logic unchanged so HOLIDAY_DATES, MONTH_NAMES and _pick_equinox_day usages remain the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/calendar/tibetan_generator.py`:
- Around line 205-208: The temporary file created as tmp_file before calling
urlretrieve(DATA_URL, tmp_file) can be left behind if urlretrieve raises; wrap
the download and replace steps in a try/finally (or try/except/finally) around
urlretrieve and tmp_file.replace(data_file) so that in the finally block you
check for tmp_file.exists() and unlink it on failure, ensuring tmp_file is
removed unless the replace succeeded; reference tmp_file, urlretrieve, DATA_URL
and data_file to locate and modify the logic.
---
Duplicate comments:
In `@scripts/calendar/tibetan_generator.py`:
- Around line 176-200: Add defensive tuple-length guards to the three extraction
functions to avoid IndexError if upstream day tuples are short: in
_extract_tibetan_holidays, _extract_blessed_rainy_day and
_extract_winter_solstice, skip any day tuple d where len(d) < 9 (or the number
of indices you access) before reading d[2], d[3], d[6], d[7], d[8]; keep the
rest of the logic unchanged so HOLIDAY_DATES, MONTH_NAMES and _pick_equinox_day
usages remain the same.
🪄 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: 877591de-7714-490b-9f41-4225c20afba4
📒 Files selected for processing (1)
scripts/calendar/tibetan_generator.py
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/calendar/tibetan_generator.py (1)
207-217:⚠️ Potential issue | 🟠 MajorRestore cached-file fallback on download failure.
Line 213 currently re-raises immediately after cleanup, so a transient fetch failure aborts generation even when
tibetan_data.txtalready exists locally.♻️ Proposed fix
def generate_data() -> None: data_file = Path(__file__).parent / DATA_FILENAME tmp_file = data_file.with_suffix(".tmp") try: urlretrieve(DATA_URL, tmp_file) - except Exception: + tmp_file.replace(data_file) + except Exception as err: if tmp_file.exists(): tmp_file.unlink() - raise - tmp_file.replace(data_file) + if data_file.exists(): + print(f"Download failed; using cached file: {data_file}") + else: + raise RuntimeError("No data file available and download failed.") from err#!/bin/bash set -euo pipefail # Verify whether generate_data has cached fallback logic inside the download exception path. rg -n -C4 'def generate_data|urlretrieve|except Exception|data_file\.exists\(|tmp_file\.replace|RuntimeError' scripts/calendar/tibetan_generator.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/calendar/tibetan_generator.py` around lines 207 - 217, The download in generate_data currently always re-raises on Exception after removing tmp_file, aborting when a cached data_file already exists; change the except block handling for urlretrieve so that after cleaning up tmp_file it checks if data_file.exists() and if so returns (or logs a warning) to use the cached DATA_FILENAME, otherwise re-raises the exception; reference generate_data, urlretrieve, tmp_file, data_file, DATA_URL and DATA_FILENAME when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/calendar/tibetan_generator.py`:
- Around line 207-217: The download in generate_data currently always re-raises
on Exception after removing tmp_file, aborting when a cached data_file already
exists; change the except block handling for urlretrieve so that after cleaning
up tmp_file it checks if data_file.exists() and if so returns (or logs a
warning) to use the cached DATA_FILENAME, otherwise re-raises the exception;
reference generate_data, urlretrieve, tmp_file, data_file, DATA_URL and
DATA_FILENAME when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7a21098-4940-438c-b7d3-c6cab1ae386a
📒 Files selected for processing (1)
scripts/calendar/tibetan_generator.py
Contributor
Author
|
@KJhellico Is the current approach of always re-downloading and failing hard on network errors acceptable, or should the generator fall back to a cached tibetan_data.txt when a download fails ? |
Collaborator
The approach should be simple: if there's a local file, we work with it; if not, we download it. And that's it. |
Contributor
Author
|
Should the download logic be simplified to use the local file if it exists, download only if it doesn't? No temp file, no fallback handling. Is this approach fine ? |
Collaborator
Yes, exactly. |
KJhellico
reviewed
Mar 16, 2026
KJhellico
reviewed
Mar 16, 2026
|
Contributor
Author
|
I've removed Winter Solstice and Blessed Rainy Day from the generator as they are solar holidays, but the existing tests in test_bhutan.py and BH.py still expect them in tibetan.py. Should these be hardcoded directly in tibetan.py, or moved into BH.py? What would be the preferred approach here? |
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
Implemented a generator script for Tibetan calendar holidays to make future updates reproducible and transparent.
This PR addresses issue #2782 by creating a data-driven generator that:
tibetan.pyformatType of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.