Update Chinese Lunisolar calendar: extend Winter Solstice support #2927
+113
−17
Conversation
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
Summary by CodeRabbit
WalkthroughAdds per-calendar winter-solstice thresholds and a new _ChineseLunisolar.winter_solstice_date(year, calendar=None) returning (date, estimated); imports Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Ruff (0.12.2)holidays/calendars/chinese.py�[1;31mruff failed�[0m 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #2927 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 296 296
Lines 17584 17593 +9
Branches 2269 2270 +1
=========================================
+ Hits 17584 17593 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/calendars/chinese.py(2 hunks)holidays/groups/chinese.py(2 hunks)tests/calendars/test_chinese.py(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2323
File: holidays/countries/macau.py:385-396
Timestamp: 2025-03-04T10:06:09.555Z
Learning: The winter solstice calculation in Macau's holiday implementation uses the same approximation method as Hong Kong's implementation and has been verified against the official Macau calendar for years 2017-2025. The method is valid for years 1952-2099.
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-04-05T04:47:27.213Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/groups/chinese.pyholidays/calendars/chinese.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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.
Applied to files:
holidays/groups/chinese.pyholidays/calendars/chinese.py
📚 Learning: 2025-08-11T13:48:45.953Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/groups/chinese.pyholidays/calendars/chinese.py
📚 Learning: 2025-07-31T15:28:12.940Z
Learnt from: KJhellico
PR: vacanza/holidays#2759
File: holidays/countries/azerbaijan.py:209-228
Timestamp: 2025-07-31T15:28:12.940Z
Learning: The _CustomCalendarType metaclass in holidays/calendars/custom.py automatically renames all public attributes (not starting with underscore) by adding the CUSTOM_ATTR_POSTFIX suffix ("_CUSTOM_CALENDAR") and deleting the original attribute. This means when defining custom calendar attributes like EID_AL_ADHA_DATES_CONFIRMED_YEARS, they should NOT include the _CUSTOM_CALENDAR suffix manually as the metaclass adds it automatically.
Applied to files:
holidays/calendars/chinese.py
🧬 Code graph analysis (2)
tests/calendars/test_chinese.py (1)
holidays/calendars/chinese.py (1)
winter_solstice_date(1362-1400)
holidays/groups/chinese.py (1)
holidays/calendars/chinese.py (1)
winter_solstice_date(1362-1400)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build distribution
- GitHub Check: Test docs build
🔇 Additional comments (3)
holidays/groups/chinese.py (2)
17-17: Import cleanup LGTM.Dropping DEC is correct after delegating Dongzhi to the calendar method.
82-84: Delegation to calendar method is the right move.Centralizes logic and keeps groups thin. No issues.
holidays/calendars/chinese.py (1)
17-17: DEC import is necessary for winter_solstice_date.All good.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/calendars/chinese.py(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-08-11T13:48:45.953Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-07-31T15:28:12.940Z
Learnt from: KJhellico
PR: vacanza/holidays#2759
File: holidays/countries/azerbaijan.py:209-228
Timestamp: 2025-07-31T15:28:12.940Z
Learning: The _CustomCalendarType metaclass in holidays/calendars/custom.py automatically renames all public attributes (not starting with underscore) by adding the CUSTOM_ATTR_POSTFIX suffix ("_CUSTOM_CALENDAR") and deleting the original attribute. This means when defining custom calendar attributes like EID_AL_ADHA_DATES_CONFIRMED_YEARS, they should NOT include the _CUSTOM_CALENDAR suffix manually as the metaclass adds it automatically.
Applied to files:
holidays/calendars/chinese.py
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (1)
holidays/calendars/chinese.py (1)
17-17: Targeted DEC import is correct.DEC is used only in winter_solstice_date; no unused month constants are introduced.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
tests/calendars/test_chinese.py (3)
26-45: Add a month guard (should always be December).Cheap extra assertion to prevent regressions.
Apply this diff inside the loop:
- self.assertEqual(self.calendar.winter_solstice_date(year)[0].day, day) + dt, _ = self.calendar.winter_solstice_date(year) + self.assertEqual(dt.day, day) + self.assertEqual(dt.month, 12)
46-63: Add a month guard (Korean calendar).Same reason as above.
Apply this diff inside the loop:
- self.assertEqual(self.calendar.winter_solstice_date(year, KOREAN_CALENDAR)[0].day, day) + dt, _ = self.calendar.winter_solstice_date(year, KOREAN_CALENDAR) + self.assertEqual(dt.day, day) + self.assertEqual(dt.month, 12)
65-85: Add a month guard (Vietnamese calendar).Same small robustness win.
Apply this diff inside the loop:
- self.assertEqual( - self.calendar.winter_solstice_date(year, VIETNAMESE_CALENDAR)[0].day, day - ) + dt, _ = self.calendar.winter_solstice_date(year, VIETNAMESE_CALENDAR) + self.assertEqual(dt.day, day) + self.assertEqual(dt.month, 12)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/calendars/chinese.py(3 hunks)tests/calendars/test_chinese.py(1 hunks)
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2631
File: tests/countries/test_sint_maarten.py:62-0
Timestamp: 2025-06-14T21:12:07.224Z
Learning: KJhellico prefers to focus on completing and reviewing the main holiday implementation code before doing detailed reviews of the corresponding test files.
📚 Learning: 2025-04-05T04:47:27.213Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-04-05T06:49:06.217Z
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-06-15T11:52:39.572Z
Learnt from: PPsyrius
PR: vacanza/holidays#2601
File: tests/countries/test_mongolia.py:128-156
Timestamp: 2025-06-15T11:52:39.572Z
Learning: In the vacanza/holidays project tests, when testing holidays that span multiple consecutive days across many years (like Mongolia's National Festival spanning July 11-13), prefer explicit for loops over complex nested generator expressions with unpacking. The explicit loops are more readable, easier to maintain, and better communicate the testing intent even though the Big O complexity is equivalent.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-08-11T13:48:45.953Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-07-31T15:28:12.940Z
Learnt from: KJhellico
PR: vacanza/holidays#2759
File: holidays/countries/azerbaijan.py:209-228
Timestamp: 2025-07-31T15:28:12.940Z
Learning: The _CustomCalendarType metaclass in holidays/calendars/custom.py automatically renames all public attributes (not starting with underscore) by adding the CUSTOM_ATTR_POSTFIX suffix ("_CUSTOM_CALENDAR") and deleting the original attribute. This means when defining custom calendar attributes like EID_AL_ADHA_DATES_CONFIRMED_YEARS, they should NOT include the _CUSTOM_CALENDAR suffix manually as the metaclass adds it automatically.
Applied to files:
holidays/calendars/chinese.py
🧬 Code graph analysis (1)
tests/calendars/test_chinese.py (2)
holidays/calendars/chinese.py (2)
_ChineseLunisolar(31-1398)winter_solstice_date(1380-1398)tests/common.py (1)
TestCase(31-370)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (3)
holidays/calendars/chinese.py (1)
17-17: DEC import is appropriate.Needed for winter_solstice_date; import set is consistent with months used in this module.
tests/calendars/test_chinese.py (2)
15-15: Good: testing via public constants.Importing KOREAN_CALENDAR and VIETNAMESE_CALENDAR keeps tests aligned with API.
86-89: LGTM on estimated-flag edges.Covers out-of-range years; optional follow-up is to assert False for 1941 and 2099.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
tests/calendars/test_chinese.py (1)
26-45: Also assert month=12 to guard regressions.Adds a cheap safety net that the solstice stays in December.
for year, day in ( @@ ): - self.assertEqual(self.calendar.winter_solstice_date(year)[0].day, day) + self.assertEqual(self.calendar.winter_solstice_date(year)[0].day, day) + self.assertEqual(self.calendar.winter_solstice_date(year)[0].month, 12)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/calendars/test_chinese.py(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-04-05T04:47:27.213Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-08-12T17:16:54.497Z
Learnt from: PPsyrius
PR: vacanza/holidays#2794
File: tests/calendars/test_julian.py:35-36
Timestamp: 2025-08-12T17:16:54.497Z
Learning: In the vacanza/holidays project calendar tests (Thai, Ethiopian, Julian, etc.), the established testing pattern for validation methods is to use simple for loops like `for year in known_data_dict:` followed by `self.assertEqual(expected, actual)` without using unittest's subTest feature. This pattern is consistently maintained across all calendar test files.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-04-05T06:49:06.217Z
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-06-15T11:52:39.572Z
Learnt from: PPsyrius
PR: vacanza/holidays#2601
File: tests/countries/test_mongolia.py:128-156
Timestamp: 2025-06-15T11:52:39.572Z
Learning: In the vacanza/holidays project tests, when testing holidays that span multiple consecutive days across many years (like Mongolia's National Festival spanning July 11-13), prefer explicit for loops over complex nested generator expressions with unpacking. The explicit loops are more readable, easier to maintain, and better communicate the testing intent even though the Big O complexity is equivalent.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-06-18T17:01:58.067Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: tests/countries/test_wallis_and_futuna.py:19-23
Timestamp: 2025-06-18T17:01:58.067Z
Learning: In the vacanza/holidays repository, the standard pattern for test class setUpClass methods is `super().setUpClass(HolidayClass)` or `super().setUpClass(HolidayClass, years=...)` where HolidayClass is passed as the first argument. This is the correct signature that matches the CommonCountryTests.setUpClass method which accepts test_class as the first parameter after cls. Pylint warnings about parameter count mismatch are false positives when comparing against TestCase.setUpClass instead of the immediate parent CommonCountryTests.setUpClass.
Applied to files:
tests/calendars/test_chinese.py
📚 Learning: 2025-06-13T15:15:25.159Z
Learnt from: KJhellico
PR: vacanza/holidays#2609
File: tests/countries/test_nauru.py:20-24
Timestamp: 2025-06-13T15:15:25.159Z
Learning: In the vacanza/holidays test suite, overriding `setUpClass` is intentionally done with the single `cls` parameter (no *args/**kwargs), so signature-mismatch lint warnings are ignored project-wide.
Applied to files:
tests/calendars/test_chinese.py
🧬 Code graph analysis (1)
tests/calendars/test_chinese.py (1)
holidays/calendars/chinese.py (1)
winter_solstice_date(1380-1398)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (1)
tests/calendars/test_chinese.py (1)
15-15: LGTM: importing new calendar constants.Good to see KOREAN_CALENDAR and VIETNAMESE_CALENDAR covered directly in tests.
arkid15r
reviewed
Sep 14, 2025
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
holidays/calendars/chinese.py (1)
1380-1399: Avoid “magic 0” default; make Dec‑23 check explicit.Prevents implicit reliance on 0 and is clearer at a glance.
Apply this diff:
- if year >= thresholds["dec_21"][year_mod]: - day = 21 - elif year <= thresholds["dec_23"].get(year_mod, 0): - day = 23 - else: - day = 22 + if year >= thresholds["dec_21"][year_mod]: + day = 21 + else: + dec23_year = thresholds["dec_23"].get(year_mod) + if dec23_year is not None and year <= dec23_year: + day = 23 + else: + day = 22
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/calendars/chinese.py(3 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2631
File: tests/countries/test_sint_maarten.py:62-0
Timestamp: 2025-06-14T21:12:07.224Z
Learning: KJhellico prefers to focus on completing and reviewing the main holiday implementation code before doing detailed reviews of the corresponding test files.
📚 Learning: 2025-04-02T18:38:35.164Z
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: tests/countries/test_guinea.py:237-239
Timestamp: 2025-04-02T18:38:35.164Z
Learning: In the vacanza/holidays project, the method assertLocalizedHolidays in country test files should be called with positional parameters rather than named parameters to maintain consistency with the rest of the codebase.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-07-24T15:21:31.632Z
Learnt from: PPsyrius
PR: vacanza/holidays#2750
File: tests/countries/test_germany.py:46-46
Timestamp: 2025-07-24T15:21:31.632Z
Learning: In the holidays project test files, the standard method name for testing the absence of holidays is `test_no_holidays`, not more descriptive names like `test_no_holidays_before_1990`. This is a consistent naming convention across country test files like France and Germany.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-08-21T04:56:03.780Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-07-10T03:36:16.461Z
Learnt from: PPsyrius
PR: vacanza/holidays#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, special holiday dates, etc.) are typically defined within the _populate_public_holidays method rather than as module-level constants. This is the established library-wide pattern and should be maintained for consistency.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-04-23T09:22:41.753Z
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:86-88
Timestamp: 2025-04-23T09:22:41.753Z
Learning: For holiday definitions in the holidays package, keep comments simple with just the holiday name (e.g., "# Independence Day.") rather than including dates or historical context, as the function names already encode the date information.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-04-03T16:58:27.175Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-08-21T05:56:33.276Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-08-11T13:48:45.953Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/calendars/chinese.py
📚 Learning: 2025-07-31T15:28:12.940Z
Learnt from: KJhellico
PR: vacanza/holidays#2759
File: holidays/countries/azerbaijan.py:209-228
Timestamp: 2025-07-31T15:28:12.940Z
Learning: The _CustomCalendarType metaclass in holidays/calendars/custom.py automatically renames all public attributes (not starting with underscore) by adding the CUSTOM_ATTR_POSTFIX suffix ("_CUSTOM_CALENDAR") and deleting the original attribute. This means when defining custom calendar attributes like EID_AL_ADHA_DATES_CONFIRMED_YEARS, they should NOT include the _CUSTOM_CALENDAR suffix manually as the metaclass adds it automatically.
Applied to files:
holidays/calendars/chinese.py
🔇 Additional comments (2)
holidays/calendars/chinese.py (2)
17-17: DEC import is appropriate.Needed for returning December dates from winter_solstice_date; aligns with “import only what’s used.”
1380-1399: Edge-year behavior verified — no change required.
WINTER_SOLSTICE_THRESHOLDS in holidays/calendars/chinese.py contains dec_23 entries for mod=3 (VIETNAMESE: 1943, CHINESE: 1947, KOREAN: 1955) and the code uses >= for dec_21 so the switch to Dec‑21 occurs on the threshold year; tests in tests/calendars/test_chinese.py assert those years/days.
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
Update Chinese Lunisolar calendar: extend Winter solstice support (for Korean and Vietnamese calendar).
North Korea has holidays related to the Winter Solstice. Vietnam doesn't have such holidays, but support has been added for consistency.
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.