Update ruff configuration: add flake8-boolean-trap (FBT) errors check
#3007
Conversation
Summary by CodeRabbit
WalkthroughStandardizes many function signatures to require keyword-only parameters (adds Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Many similar signature changes (low per-file logic density) but spread across many modules and tests; verify call-site adaptations, type annotations in generator scripts, and lint config changes. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (28)
holidays/countries/nepal.py(1 hunks)holidays/countries/norway.py(1 hunks)holidays/countries/rwanda.py(2 hunks)holidays/countries/sweden.py(1 hunks)holidays/countries/taiwan.py(1 hunks)holidays/countries/tanzania.py(1 hunks)holidays/countries/trinidad_and_tobago.py(1 hunks)holidays/countries/ukraine.py(1 hunks)holidays/countries/united_states.py(1 hunks)holidays/groups/buddhist.py(2 hunks)holidays/groups/chinese.py(2 hunks)holidays/groups/eastern.py(3 hunks)holidays/groups/hindu.py(3 hunks)holidays/groups/international.py(1 hunks)holidays/groups/islamic.py(2 hunks)holidays/groups/mongolian.py(2 hunks)holidays/groups/sinhala.py(3 hunks)holidays/groups/tibetan.py(2 hunks)holidays/holiday_base.py(1 hunks)holidays/observed_holiday_base.py(3 hunks)holidays/registry.py(1 hunks)pyproject.toml(1 hunks)scripts/calendar/asian_generator.py(1 hunks)scripts/calendar/mongolian_generator.py(6 hunks)tests/common.py(1 hunks)tests/financial/test_ny_stock_exchange.py(1 hunks)tests/test_holiday_base.py(1 hunks)tests/test_ical.py(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2702
File: holidays/countries/__init__.py:65-69
Timestamp: 2025-07-05T20:26:31.219Z
Learning: The vacanza/holidays project uses ruff as the primary linter for code checking and linting. Flake8 suppression comments are not needed since the project defers to ruff's rules, and existing `ruff: noqa` comments should be sufficient for handling linting warnings.
📚 Learning: 2025-06-13T12:18:03.539Z
Learnt from: PPsyrius
PR: vacanza/holidays#2614
File: holidays/countries/guyana.py:78-90
Timestamp: 2025-06-13T12:18:03.539Z
Learning: The holidays codebase now uses the constructor signature pattern `__init__(self, *args, islamic_show_estimated: bool = True, **kwargs)` across country classes.
Applied to files:
holidays/countries/norway.pyholidays/groups/tibetan.pyholidays/groups/sinhala.pyholidays/groups/chinese.pyholidays/groups/international.pyholidays/groups/mongolian.pyholidays/groups/islamic.pyholidays/groups/buddhist.pyholidays/groups/hindu.py
📚 Learning: 2025-08-19T14:42:45.111Z
Learnt from: PPsyrius
PR: vacanza/holidays#2828
File: holidays/countries/australia.py:813-816
Timestamp: 2025-08-19T14:42:45.111Z
Learning: In the holidays library, when using _add_observed() method calls, there's no need to explicitly pass the rule parameter if it matches the class's default observed_rule. The default rule is set in __init__ specifically to avoid redundant explicit declarations, and should only be overridden when a different rule is needed.
Applied to files:
holidays/countries/taiwan.pyholidays/countries/tanzania.py
📚 Learning: 2025-08-28T02:34:11.657Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/groups/eastern.pyholidays/groups/sinhala.py
📚 Learning: 2025-04-25T20:27:59.086Z
Learnt from: KJhellico
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:85-92
Timestamp: 2025-04-25T20:27:59.086Z
Learning: The `_populate_observed` method in holiday classes should maintain the same signature as the parent class `ObservedHolidayBase`, even if specific child class implementations don't use all parameters.
Applied to files:
tests/test_holiday_base.pyholidays/countries/trinidad_and_tobago.pyholidays/holiday_base.pyholidays/observed_holiday_base.py
📚 Learning: 2025-08-22T19:06:04.303Z
Learnt from: KJhellico
PR: vacanza/holidays#2850
File: holidays/countries/christmas_island.py:75-80
Timestamp: 2025-08-22T19:06:04.303Z
Learning: Christmas Island's docstring for the `islamic_show_estimated` parameter follows the exact same format used consistently across all countries with Islamic holidays in the codebase: "Whether to add 'estimated' label to Islamic holidays name if holiday date is estimated." This is the standard, established pattern and should not be changed.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-07-10T15:55:34.523Z
Learnt from: KJhellico
PR: vacanza/holidays#2706
File: holidays/countries/cayman_islands.py:50-55
Timestamp: 2025-07-10T15:55:34.523Z
Learning: The `islamic_show_estimated` parameter in country class constructors is only needed for countries that implement Islamic holidays (inherit from IslamicHolidays or _CustomIslamicHolidays groups). Countries with only Christian and secular holidays do not need this parameter.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-08-25T04:28:02.061Z
Learnt from: PPsyrius
PR: vacanza/holidays#2848
File: tests/countries/test_somalia.py:44-127
Timestamp: 2025-08-25T04:28:02.061Z
Learning: In the holidays library, Islamic holiday tests use `self.no_estimated_holidays = Country(years=years, islamic_show_estimated=False)` as the library-wide standard approach to simplify test cases. This pattern is intentional and preferred over testing estimated labels.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-08-19T21:00:47.849Z
Learnt from: KJhellico
PR: vacanza/holidays#2831
File: holidays/countries/south_sudan.py:84-88
Timestamp: 2025-08-19T21:00:47.849Z
Learning: In the holidays library, Islamic holidays use dedicated methods for additional days (like `_add_eid_al_fitr_day_two`, `_add_eid_al_adha_day_two`) rather than parameters. The methods don't accept a `days` parameter - each day has its own specific method.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-06-04T10:09:28.732Z
Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2593
File: holidays/calendars/islamic.py:3993-3994
Timestamp: 2025-06-04T10:09:28.732Z
Learning: In the holidays/calendars/islamic.py file, the Islamic calendar date methods in the _IslamicLunar class (like ali_al_rida_death_dates, ashura_dates, etc.) follow a consistent pattern of being single-line methods that return self._get_holiday(CONSTANT, year) without docstrings. New methods should follow this same pattern for consistency.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-06-16T11:17:34.671Z
Learnt from: PPsyrius
PR: vacanza/holidays#2637
File: holidays/countries/singapore.py:49-50
Timestamp: 2025-06-16T11:17:34.671Z
Learning: For the vacanza/holidays project, defer to ruff's line length rules rather than pylint's C0301 warnings. If ruff passes in pre-commit tests, line length is acceptable regardless of pylint warnings about exceeding 100 characters.
Applied to files:
pyproject.toml
🧬 Code graph analysis (8)
holidays/countries/rwanda.py (4)
holidays/countries/taiwan.py (1)
_populate_observed(114-138)holidays/countries/tanzania.py (1)
_populate_observed(139-164)holidays/countries/trinidad_and_tobago.py (1)
_populate_observed(90-97)holidays/observed_holiday_base.py (1)
_populate_observed(220-231)
holidays/countries/tanzania.py (6)
holidays/observed_holiday_base.py (1)
_add_observed(144-198)holidays/countries/tonga.py (1)
_add_observed(96-99)holidays/countries/jersey.py (1)
_add_observed(63-69)holidays/countries/guernsey.py (1)
_add_observed(56-62)holidays/countries/israel.py (1)
_add_observed(52-55)holidays/countries/angola.py (1)
_add_observed(65-71)
tests/test_holiday_base.py (4)
holidays/observed_holiday_base.py (1)
_add_observed(144-198)holidays/countries/tonga.py (1)
_add_observed(96-99)holidays/countries/israel.py (1)
_add_observed(52-55)holidays/countries/angola.py (1)
_add_observed(65-71)
holidays/countries/trinidad_and_tobago.py (2)
holidays/countries/rwanda.py (1)
_populate_observed(63-80)holidays/observed_holiday_base.py (1)
_populate_observed(220-231)
holidays/countries/united_states.py (1)
holidays/countries/ukraine.py (1)
_populate_common(57-151)
holidays/observed_holiday_base.py (6)
holidays/countries/rwanda.py (1)
_populate_observed(63-80)holidays/countries/taiwan.py (1)
_populate_observed(114-138)holidays/countries/tanzania.py (1)
_populate_observed(139-164)holidays/countries/trinidad_and_tobago.py (1)
_populate_observed(90-97)holidays/countries/bulgaria.py (1)
_populate_observed(64-70)holidays/countries/japan.py (1)
_populate_observed(58-82)
holidays/countries/ukraine.py (1)
holidays/countries/united_states.py (1)
_populate_common(252-363)
scripts/calendar/mongolian_generator.py (1)
scripts/calendar/asian_generator.py (1)
generate_data(388-427)
🪛 Ruff (0.14.0)
holidays/countries/sweden.py
42-42: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
42-42: Missing type annotation for *args
(ANN002)
42-42: Missing type annotation for **kwargs
(ANN003)
holidays/countries/norway.py
87-87: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
87-87: Missing type annotation for *args
(ANN002)
87-87: Missing type annotation for **kwargs
(ANN003)
holidays/countries/rwanda.py
63-63: Unused method argument: multiple
(ARG002)
holidays/countries/nepal.py
77-77: Missing return type annotation for private function _add_non_continuous_holidays
Add return type annotation: None
(ANN202)
holidays/countries/trinidad_and_tobago.py
90-90: Unused method argument: multiple
(ARG002)
holidays/groups/international.py
215-215: Missing return type annotation for private function _add_world_war_two_victory_day
(ANN202)
holidays/holiday_base.py
811-811: Missing return type annotation for private function _add_special_holidays
Add return type annotation: None
(ANN202)
holidays/countries/united_states.py
252-252: Missing return type annotation for private function _populate_common
Add return type annotation: None
(ANN202)
tests/financial/test_ny_stock_exchange.py
428-428: Missing return type annotation for private function _make_special_holiday_list
(ANN202)
holidays/countries/ukraine.py
57-57: Missing return type annotation for private function _populate_common
Add return type annotation: None
(ANN202)
⏰ 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 macos-latest
🔇 Additional comments (35)
pyproject.toml (1)
59-63: LGTM! Configuration changes align with PR objectives.The addition of FBT (flake8-boolean-trap) to the lint rules and the per-file-ignores for public API files correctly implements the approach described in the PR: enforcing keyword-only parameters for internal methods while preserving the public API surface.
holidays/groups/buddhist.py (2)
25-25: LGTM! Keyword-only parameter enforcement.The
show_estimatedparameter is now keyword-only, aligning with the PR objectives to prevent boolean traps in internal methods.
39-39: LGTM! Call site updated correctly.The call to
_add_eastern_calendar_holidaynow uses keyword argument forshow_estimated, consistent with the signature change at line 25.scripts/calendar/asian_generator.py (1)
288-288: LGTM! Keyword-only parameter enforcement.The
leapparameter is now keyword-only, preventing boolean trap issues. Call sites at lines 316-317 already use keyword arguments, so this change is non-breaking.holidays/groups/chinese.py (2)
26-26: LGTM! Keyword-only parameter enforcement.The
show_estimatedandcalendarparameters are now keyword-only, aligning with the PR objectives to prevent boolean traps in internal methods.
95-98: LGTM! Call site updated correctly.The call to
_add_eastern_calendar_holidaynow uses keyword arguments for bothshow_estimatedanddays_delta, consistent with the signature change at line 26.holidays/groups/eastern.py (2)
29-29: LGTM! Keyword-only parameters prevent boolean traps.Adding the keyword-only separator ensures
show_estimatedanddays_deltamust be passed by name, preventing argument misplacement and improving code clarity.Also applies to: 57-57
72-72: LGTM! Internal call correctly uses keyword arguments.The call to
_add_eastern_calendar_holidayproperly passes both parameters as keywords, consistent with the updated signature.holidays/groups/islamic.py (2)
28-28: LGTM! Enforces keyword-only parameters in constructor.Making
show_estimatedandcalendar_delta_dayskeyword-only prevents accidental positional argument errors.
289-292: LGTM! Internal call aligns with keyword-only enforcement.The call to
_add_eastern_calendar_holiday_setcorrectly passes all arguments as keywords, matching the updated base method signature.holidays/groups/mongolian.py (2)
25-25: LGTM! Keyword-only parameter enforced.Consistent with the pattern across other calendar group classes.
40-43: LGTM! Call site updated correctly.Parameters passed as keywords align with the base helper signature.
holidays/groups/tibetan.py (2)
25-25: LGTM! Consistent keyword-only enforcement.Matches the pattern established across all calendar group classes.
39-42: LGTM! Keyword arguments properly applied.Internal call correctly uses keyword arguments for both parameters.
holidays/groups/sinhala.py (2)
35-35: LGTM! Keyword-only parameter enforced.Consistent implementation across calendar groups.
49-49: LGTM! Both internal helpers use keyword arguments.Calls to
_add_eastern_calendar_holidayand_add_eastern_calendar_holiday_setcorrectly pass parameters as keywords.Also applies to: 62-65
holidays/groups/hindu.py (2)
26-26: LGTM! Keyword-only parameter enforced.Matches the pattern across all calendar group classes.
41-44: LGTM! Internal calls correctly use keyword arguments.Both helper methods properly forward parameters as keywords to the base eastern calendar helpers.
Also applies to: 57-60
scripts/calendar/mongolian_generator.py (2)
44-44: LGTM! Comprehensive type annotations and keyword-only enforcement.Adding type hints throughout the module improves maintainability and catches type errors early. Making
is_leapkeyword-only intrue_month,julian_day, andmongolian_to_gregorianprevents boolean trap issues.Also applies to: 48-48, 53-53, 58-58, 64-64, 78-78, 92-92, 102-102, 108-108, 119-119, 123-123, 127-127, 155-155
103-103: LGTM! All call sites updated to use keyword arguments.Internal calls correctly pass
is_leapas a keyword argument, consistent with the updated signatures.Also applies to: 120-120, 124-124, 128-128
holidays/countries/taiwan.py (1)
137-137: LGTM! Rule parameter passed as keyword.Using
rule=...syntax improves clarity, especially with the conditional expression, and aligns with the broader keyword-only parameter enforcement across the codebase.tests/test_ical.py (1)
377-377: LGTM!Clear improvement converting the boolean positional argument to a named keyword. Makes the intent explicit.
holidays/countries/tanzania.py (1)
164-164: LGTM!Correctly passes
ruleas a keyword argument to match the parent class signature where it's keyword-only.holidays/countries/rwanda.py (1)
63-63: LGTM!The signature change correctly makes
multiplekeyword-only, maintaining compatibility with the parent class signature pattern.holidays/registry.py (1)
353-359: LGTM!Making these parameters keyword-only is safe—all existing call sites already use keyword arguments.
holidays/holiday_base.py (1)
811-811: LGTM!Making
observedkeyword-only improves clarity for this internal helper method.tests/test_holiday_base.py (1)
43-43: LGTM!Making boolean parameters keyword-only in the test stub correctly demonstrates the expected usage pattern.
holidays/countries/norway.py (1)
87-87: LGTM! Note the API change.The signature change makes
include_sundayskeyword-only, which is a breaking change. Callers usingNorway(True)must update toNorway(include_sundays=True). This aligns with the PR objective and improves API clarity.holidays/countries/nepal.py (1)
77-77: LGTM!Making
is_workdaykeyword-only is safe—the call site at line 298 already uses the keyword argument form.holidays/countries/trinidad_and_tobago.py (1)
90-90: LGTM! Signature correctly matches parent class.The keyword-only
multipleparameter aligns with the parentObservedHolidayBase._populate_observedsignature. The unused parameter warning from static analysis is expected since this override maintains the parent signature without using all parameters.Based on learnings.
tests/common.py (1)
73-74: LGTM! Keyword-only parameters improve test clarity.Enforcing keyword arguments for the boolean
expand_itemsandraise_on_emptyparameters prevents boolean trap issues in test code.tests/financial/test_ny_stock_exchange.py (1)
428-428: LGTM! Keyword-only parameters prevent boolean trap.Making
weekendskeyword-only improves readability at call sites. The missing return type annotation flagged by static analysis is a pre-existing issue outside the scope of this PR.holidays/observed_holiday_base.py (3)
144-152: LGTM! Keyword-only parameters prevent boolean trap.Enforcing keyword arguments for
rule,force_observed, andshow_observed_labelimproves readability and prevents positional boolean argument issues. Existing calls throughout the codebase already use keyword syntax.
200-207: LGTM! Consistent with_add_observedsignature.Making parameters keyword-only maintains consistency with the
_add_observedmethod and prevents boolean trap issues.
220-220: LGTM! Base signature for child class overrides.This establishes the keyword-only convention for the
multipleparameter, which child classes likeTrinidadAndTobagocorrectly maintain.
d5c3adb to
bfc564a
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3007 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 306 306
Lines 18076 18076
Branches 2334 2334
=========================================
Hits 18076 18076 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
holidays/countries/ukraine.py (1)
57-57: Good change. Consider adding return type annotation.Making
is_martial_lawkeyword-only is correct for this internal method. Both call sites (lines 154, 157) already pass it as a keyword argument.As noted in a previous review, consider adding
-> Noneto the signature for completeness:- def _populate_common(self, *, is_martial_law: bool = False): + def _populate_common(self, *, is_martial_law: bool = False) -> None:holidays/groups/international.py (1)
215-215: Keyword-only parameter correctly implemented.The
*separator properly enforces keyword-only usage foris_western, preventing boolean trap issues.Note: The missing return type annotation was already flagged in a previous review.
holidays/countries/united_states.py (1)
252-252: Keyword-only parameter correctly implemented.The
*separator properly enforces keyword-only usage forinclude_federal, preventing boolean trap issues. The call site at line 1324 already uses the keyword argument pattern.Note: The missing return type annotation was already flagged in a previous review.
holidays/countries/sweden.py (1)
42-42: LGTM – constructor signature updated to keyword-only parameter.The
include_sundaysparameter is now keyword-only, preventing boolean trap issues. Users must now callSweden(include_sundays=True)instead ofSweden(True).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (30)
holidays/countries/azerbaijan.py(1 hunks)holidays/countries/israel.py(1 hunks)holidays/countries/nepal.py(1 hunks)holidays/countries/norway.py(1 hunks)holidays/countries/rwanda.py(2 hunks)holidays/countries/sweden.py(1 hunks)holidays/countries/taiwan.py(1 hunks)holidays/countries/tanzania.py(1 hunks)holidays/countries/trinidad_and_tobago.py(1 hunks)holidays/countries/ukraine.py(1 hunks)holidays/countries/united_states.py(1 hunks)holidays/groups/buddhist.py(2 hunks)holidays/groups/chinese.py(2 hunks)holidays/groups/eastern.py(3 hunks)holidays/groups/hindu.py(3 hunks)holidays/groups/international.py(1 hunks)holidays/groups/islamic.py(2 hunks)holidays/groups/mongolian.py(2 hunks)holidays/groups/sinhala.py(3 hunks)holidays/groups/tibetan.py(2 hunks)holidays/holiday_base.py(1 hunks)holidays/observed_holiday_base.py(3 hunks)holidays/registry.py(1 hunks)pyproject.toml(1 hunks)scripts/calendar/asian_generator.py(1 hunks)scripts/calendar/mongolian_generator.py(6 hunks)tests/common.py(1 hunks)tests/financial/test_ny_stock_exchange.py(1 hunks)tests/test_holiday_base.py(1 hunks)tests/test_ical.py(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2702
File: holidays/countries/__init__.py:65-69
Timestamp: 2025-07-05T20:26:31.219Z
Learning: The vacanza/holidays project uses ruff as the primary linter for code checking and linting. Flake8 suppression comments are not needed since the project defers to ruff's rules, and existing `ruff: noqa` comments should be sufficient for handling linting warnings.
📚 Learning: 2025-06-13T12:18:03.539Z
Learnt from: PPsyrius
PR: vacanza/holidays#2614
File: holidays/countries/guyana.py:78-90
Timestamp: 2025-06-13T12:18:03.539Z
Learning: The holidays codebase now uses the constructor signature pattern `__init__(self, *args, islamic_show_estimated: bool = True, **kwargs)` across country classes.
Applied to files:
holidays/countries/norway.pyholidays/groups/islamic.pyholidays/groups/mongolian.pyholidays/groups/tibetan.pyholidays/groups/buddhist.pyholidays/groups/hindu.pyholidays/groups/chinese.pyholidays/groups/sinhala.py
📚 Learning: 2025-04-25T20:27:59.086Z
Learnt from: KJhellico
PR: vacanza/holidays#2402
File: holidays/countries/trinidad_and_tobago.py:85-92
Timestamp: 2025-04-25T20:27:59.086Z
Learning: The `_populate_observed` method in holiday classes should maintain the same signature as the parent class `ObservedHolidayBase`, even if specific child class implementations don't use all parameters.
Applied to files:
holidays/observed_holiday_base.pyholidays/holiday_base.pyholidays/countries/trinidad_and_tobago.py
📚 Learning: 2025-08-19T14:42:45.111Z
Learnt from: PPsyrius
PR: vacanza/holidays#2828
File: holidays/countries/australia.py:813-816
Timestamp: 2025-08-19T14:42:45.111Z
Learning: In the holidays library, when using _add_observed() method calls, there's no need to explicitly pass the rule parameter if it matches the class's default observed_rule. The default rule is set in __init__ specifically to avoid redundant explicit declarations, and should only be overridden when a different rule is needed.
Applied to files:
holidays/observed_holiday_base.pyholidays/countries/taiwan.pyholidays/countries/tanzania.pyholidays/countries/azerbaijan.pyholidays/countries/israel.py
📚 Learning: 2025-07-10T15:55:34.523Z
Learnt from: KJhellico
PR: vacanza/holidays#2706
File: holidays/countries/cayman_islands.py:50-55
Timestamp: 2025-07-10T15:55:34.523Z
Learning: The `islamic_show_estimated` parameter in country class constructors is only needed for countries that implement Islamic holidays (inherit from IslamicHolidays or _CustomIslamicHolidays groups). Countries with only Christian and secular holidays do not need this parameter.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-08-22T19:06:04.303Z
Learnt from: KJhellico
PR: vacanza/holidays#2850
File: holidays/countries/christmas_island.py:75-80
Timestamp: 2025-08-22T19:06:04.303Z
Learning: Christmas Island's docstring for the `islamic_show_estimated` parameter follows the exact same format used consistently across all countries with Islamic holidays in the codebase: "Whether to add 'estimated' label to Islamic holidays name if holiday date is estimated." This is the standard, established pattern and should not be changed.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-08-25T04:28:02.061Z
Learnt from: PPsyrius
PR: vacanza/holidays#2848
File: tests/countries/test_somalia.py:44-127
Timestamp: 2025-08-25T04:28:02.061Z
Learning: In the holidays library, Islamic holiday tests use `self.no_estimated_holidays = Country(years=years, islamic_show_estimated=False)` as the library-wide standard approach to simplify test cases. This pattern is intentional and preferred over testing estimated labels.
Applied to files:
holidays/groups/islamic.py
📚 Learning: 2025-06-16T11:17:34.671Z
Learnt from: PPsyrius
PR: vacanza/holidays#2637
File: holidays/countries/singapore.py:49-50
Timestamp: 2025-06-16T11:17:34.671Z
Learning: For the vacanza/holidays project, defer to ruff's line length rules rather than pylint's C0301 warnings. If ruff passes in pre-commit tests, line length is acceptable regardless of pylint warnings about exceeding 100 characters.
Applied to files:
pyproject.toml
📚 Learning: 2025-08-28T02:34:11.657Z
Learnt from: PPsyrius
PR: vacanza/holidays#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.
Applied to files:
holidays/groups/eastern.py
📚 Learning: 2025-07-09T20:27:37.760Z
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: holidays/countries/christmas_island.py:110-112
Timestamp: 2025-07-09T20:27:37.760Z
Learning: In Christmas Island, ANZAC Day (April 25) follows the same observed holiday rules as other holidays, using the SAT_SUN_TO_NEXT_MON rule to move to Monday when it falls on a weekend. The implementation correctly uses `_add_observed` wrapper around `_add_anzac_day`.
Applied to files:
holidays/countries/azerbaijan.py
🧬 Code graph analysis (10)
holidays/observed_holiday_base.py (7)
holidays/countries/rwanda.py (1)
_populate_observed(63-80)holidays/countries/taiwan.py (1)
_populate_observed(114-138)holidays/countries/tanzania.py (1)
_populate_observed(139-164)holidays/countries/trinidad_and_tobago.py (1)
_populate_observed(90-97)holidays/countries/bulgaria.py (1)
_populate_observed(64-70)holidays/countries/south_korea.py (1)
_populate_observed(107-121)holidays/countries/japan.py (1)
_populate_observed(58-82)
holidays/countries/rwanda.py (3)
holidays/countries/taiwan.py (1)
_populate_observed(114-138)holidays/countries/trinidad_and_tobago.py (1)
_populate_observed(90-97)holidays/observed_holiday_base.py (1)
_populate_observed(220-231)
holidays/countries/tanzania.py (3)
holidays/countries/israel.py (1)
_add_observed(52-55)holidays/observed_holiday_base.py (1)
_add_observed(144-198)holidays/countries/tonga.py (1)
_add_observed(96-99)
scripts/calendar/mongolian_generator.py (1)
scripts/calendar/asian_generator.py (1)
generate_data(388-427)
holidays/countries/united_states.py (1)
holidays/countries/ukraine.py (1)
_populate_common(57-151)
holidays/countries/azerbaijan.py (6)
holidays/countries/israel.py (1)
_add_observed(52-55)holidays/observed_holiday_base.py (1)
_add_observed(144-198)holidays/countries/tonga.py (1)
_add_observed(96-99)holidays/countries/guernsey.py (1)
_add_observed(56-62)holidays/countries/jersey.py (1)
_add_observed(63-69)holidays/countries/angola.py (1)
_add_observed(65-71)
holidays/countries/israel.py (3)
holidays/observed_holiday_base.py (1)
_add_observed(144-198)holidays/countries/tonga.py (1)
_add_observed(96-99)holidays/countries/angola.py (1)
_add_observed(65-71)
holidays/countries/ukraine.py (1)
holidays/countries/united_states.py (1)
_populate_common(252-363)
holidays/countries/trinidad_and_tobago.py (2)
holidays/countries/rwanda.py (1)
_populate_observed(63-80)holidays/observed_holiday_base.py (1)
_populate_observed(220-231)
tests/test_holiday_base.py (4)
holidays/countries/israel.py (1)
_add_observed(52-55)holidays/observed_holiday_base.py (1)
_add_observed(144-198)holidays/countries/tonga.py (1)
_add_observed(96-99)holidays/countries/angola.py (1)
_add_observed(65-71)
🪛 Ruff (0.14.0)
holidays/groups/international.py
215-215: Missing return type annotation for private function _add_world_war_two_victory_day
(ANN202)
holidays/countries/norway.py
87-87: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
87-87: Missing type annotation for *args
(ANN002)
87-87: Missing type annotation for **kwargs
(ANN003)
tests/financial/test_ny_stock_exchange.py
428-428: Missing return type annotation for private function _make_special_holiday_list
(ANN202)
holidays/countries/rwanda.py
63-63: Unused method argument: multiple
(ARG002)
holidays/countries/nepal.py
77-77: Missing return type annotation for private function _add_non_continuous_holidays
Add return type annotation: None
(ANN202)
holidays/countries/united_states.py
252-252: Missing return type annotation for private function _populate_common
Add return type annotation: None
(ANN202)
holidays/countries/sweden.py
42-42: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
42-42: Missing type annotation for *args
(ANN002)
42-42: Missing type annotation for **kwargs
(ANN003)
holidays/holiday_base.py
811-811: Missing return type annotation for private function _add_special_holidays
Add return type annotation: None
(ANN202)
holidays/countries/ukraine.py
57-57: Missing return type annotation for private function _populate_common
Add return type annotation: None
(ANN202)
holidays/countries/trinidad_and_tobago.py
90-90: Unused method argument: multiple
(ARG002)
⏰ 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.14 on windows-latest
🔇 Additional comments (37)
scripts/calendar/asian_generator.py (1)
288-288: No externallunar_to_grecalls detected; keyword-onlyleapparameter is safe.tests/test_ical.py (1)
377-377: Good change for clarity.Converting the positional boolean to a keyword argument (
keepends=True) makes the intent explicit and aligns with the PR's objective to eliminate boolean traps.pyproject.toml (1)
59-63: LGTM! Configuration aligns with PR objectives.The addition of "FBT" to lint rules and the per-file-ignores for public API modules (holiday_base.py, ical.py, registry.py, utils.py) correctly implements the stated goal of enforcing boolean trap checks on internal methods while preserving the public API.
holidays/countries/trinidad_and_tobago.py (1)
90-90: LGTM! Signature correctly matches parent class.Making
multiplekeyword-only aligns with the parentObservedHolidayBase._populate_observedsignature. The unused parameter warning from static analysis is expected—this override implements custom logic that doesn't require themultipleparameter, but maintains signature compatibility as per the learning note.Based on learnings
tests/common.py (1)
73-74: Good improvement for test clarity.Making these parameters keyword-only in the test utility method ensures all callers must explicitly name them, reducing ambiguity when reading test code.
tests/test_holiday_base.py (1)
43-43: LGTM!Making boolean parameters keyword-only in the test stub aligns with the production code pattern. Call sites at lines 56 and 60 already use keyword arguments.
scripts/calendar/mongolian_generator.py (1)
44-129: Excellent comprehensive update.The addition of type annotations and conversion of
is_leapto a keyword-only parameter throughout the call chain (lines 58, 102, 119) significantly improves code clarity. All internal call sites correctly updated to passis_leap=...explicitly.holidays/countries/taiwan.py (1)
137-137: LGTM!The keyword argument usage for
rulecorrectly aligns with the project-wide shift to keyword-only parameters in_add_observedcalls.holidays/countries/israel.py (1)
52-53: LGTM!The keyword argument for
rulein thesuper()._add_observed()call correctly matches the keyword-only parameter enforcement in the base class.holidays/countries/tanzania.py (1)
164-164: LGTM!The keyword argument usage for
rulecorrectly implements the keyword-only parameter pattern.holidays/countries/azerbaijan.py (1)
175-175: LGTM!The keyword argument for
rulealigns with the keyword-only parameter enforcement across the codebase.holidays/countries/rwanda.py (2)
63-63: LGTM!The
*separator correctly enforces keyword-only usage for themultipleparameter, maintaining signature consistency with the base class.
79-79: LGTM!The keyword argument for
ruleproperly implements the conditional observed rule selection for special cases.holidays/groups/islamic.py (2)
28-28: LGTM!The
*separator correctly enforces keyword-only usage forshow_estimatedandcalendar_delta_days, preventing boolean trap issues in the constructor.
289-292: LGTM!The keyword arguments for
show_estimatedanddays_deltacorrectly match the keyword-only parameter enforcement in the base class method.holidays/holiday_base.py (1)
811-811: LGTM – keyword-only parameter enforced for internal method.The signature change correctly makes
observedkeyword-only, preventing boolean trap issues in internal calls.holidays/groups/buddhist.py (2)
25-25: LGTM – constructor signature updated correctly.The
show_estimatedparameter is now keyword-only, aligning with the PR's objective to prevent boolean traps.
39-39: LGTM – internal call updated to keyword argument.The call to
_add_eastern_calendar_holidaycorrectly passesshow_estimatedas a keyword argument, consistent with the constructor change.holidays/groups/tibetan.py (2)
25-25: LGTM – constructor signature updated correctly.The
show_estimatedparameter is now keyword-only, consistent with the PR's objective to prevent boolean traps.
39-42: LGTM – internal call updated with keyword arguments.The call to
_add_eastern_calendar_holidaynow uses keyword arguments for bothshow_estimatedanddays_delta, improving call-site clarity.holidays/registry.py (1)
353-359: LGTM – keyword-only parameters enforced for internal method.The signature change correctly makes
include_aliases,max_code_length, andmin_code_lengthkeyword-only. Call sites at lines 378-381 and 391-395 already use keyword arguments, so no updates needed.holidays/countries/norway.py (1)
87-87: LGTM – constructor signature updated to keyword-only parameter.The
include_sundaysparameter is now keyword-only, preventing boolean trap issues. Users must now callNorway(include_sundays=False)instead ofNorway(False).holidays/groups/chinese.py (2)
26-26: LGTM – constructor signature updated correctly.The
show_estimatedandcalendarparameters are now keyword-only, aligning with the PR's objective to prevent boolean traps.
95-98: LGTM – internal call updated with keyword arguments.The call to
_add_eastern_calendar_holidaynow uses keyword arguments forshow_estimatedanddays_delta, improving call-site clarity.holidays/countries/nepal.py (1)
77-77: LGTM – keyword-only parameter enforced for internal method.The signature change correctly makes
is_workdaykeyword-only, preventing boolean trap issues. The call site at line 298 already uses the keyword argument form.holidays/groups/mongolian.py (2)
25-27: LGTM! Keyword-only parameter enforcement added correctly.The
*delimiter properly enforces keyword-only usage forshow_estimated, preventing boolean trap issues.
40-44: Internal call correctly updated to use keyword arguments.The call to
_add_eastern_calendar_holidayproperly passesshow_estimatedanddays_deltaas keyword arguments, aligning with the keyword-only signature in the base class.holidays/groups/sinhala.py (2)
35-37: LGTM! Consistent keyword-only parameter pattern.The
__init__signature change matches the pattern across other calendar group classes.
48-50: Internal calls properly updated to keyword arguments.Both
_add_eastern_calendar_holidayand_add_eastern_calendar_holiday_setcalls correctly use keyword arguments forshow_estimatedanddays_delta.Also applies to: 61-66
holidays/groups/hindu.py (2)
26-28: LGTM! Keyword-only parameter enforcement added.The signature change is consistent with the other calendar group classes in this PR.
40-45: Internal method calls correctly updated.Both
_add_hindu_calendar_holidayand_add_hindu_calendar_holiday_setproperly passshow_estimatedanddays_deltaas keyword arguments.Also applies to: 56-61
holidays/groups/eastern.py (3)
25-32: LGTM! Base class signature enforces keyword-only parameters.Adding
*beforeshow_estimatedin the base class method ensures all subclasses (Mongolian, Sinhala, Hindu, etc.) must use keyword arguments, preventing boolean trap issues.
53-60: LGTM! Set variant also enforces keyword-only parameters.Consistent with the single-holiday variant, this enforces keyword arguments for
show_estimatedanddays_delta.
67-75: Internal call correctly updated to keyword arguments.The call to
_add_eastern_calendar_holidaywithin this method properly uses keyword arguments, maintaining consistency with the signature changes.holidays/observed_holiday_base.py (3)
144-152: LGTM! Keyword-only enforcement for observed rule configuration.Making
rule,force_observed, andshow_observed_labelkeyword-only prevents boolean trap issues. The change is compatible with existing call sites based on the relevant code snippets provided.
200-207: LGTM! Consistent keyword-only pattern.The
_move_holidaysignature change mirrors_add_observed, maintaining consistency. Line 209 already passesrule=ruleas a keyword argument.
220-231: LGTM! Multiple parameter made keyword-only.The
multipleboolean parameter is now keyword-only, preventing boolean trap issues. Existing call sites in the relevant code snippets already usemultiple=Falseas a keyword argument.
Yes, I think, return types annotations better to add in separate PR, and for all methods, not selectively. (Here, #750 looks at us with curiosity 😀) |
Proposed change
Update ruff configuration: add
flake8-boolean-trap(FBT) errors check.Make internal methods keyword-only for boolean arguments to avoid boolean traps; no changes in public API methods (exclusion added).
Resolves #2967.
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.