-
-
Notifications
You must be signed in to change notification settings - Fork 554
Update Canada holidays: adjust introduction year of National Aboriginal Day in NT #2804
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary by CodeRabbit
WalkthroughAdjusted Canada holiday logic to require year >= 2001 for Northwest Territories National Aboriginal Day, removed NT snapshot entries for 1996–2000, and updated tests to reflect the new 2001 start year. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (12)📓 Common learnings
📚 Learning: 2025-05-06T21:07:11.577Z
Applied to files:
📚 Learning: 2025-04-05T04:47:27.213Z
Applied to files:
📚 Learning: 2025-07-24T15:21:31.632Z
Applied to files:
📚 Learning: 2025-04-05T06:49:06.217Z
Applied to files:
📚 Learning: 2025-07-09T21:16:35.145Z
Applied to files:
📚 Learning: 2025-05-09T18:36:09.607Z
Applied to files:
📚 Learning: 2025-07-02T18:17:53.342Z
Applied to files:
📚 Learning: 2025-06-15T11:52:39.572Z
Applied to files:
📚 Learning: 2025-07-02T18:21:59.302Z
Applied to files:
📚 Learning: 2025-04-02T18:38:35.164Z
Applied to files:
📚 Learning: 2025-06-14T20:43:15.370Z
Applied to files:
🧬 Code Graph Analysis (1)tests/countries/test_canada.py (1)
⏰ 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)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #2804 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 284 284
Lines 17049 17049
Branches 2253 2253
=========================================
Hits 17049 17049 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/canada.py
(2 hunks)snapshots/countries/CA_NT.json
(0 hunks)tests/countries/test_canada.py
(1 hunks)
💤 Files with no reviewable changes (1)
- snapshots/countries/CA_NT.json
🧰 Additional context used
🧠 Learnings (16)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: tests/countries/test_finland.py:23-26
Timestamp: 2025-05-09T18:36:09.607Z
Learning: The holidays project prioritizes complete historical coverage in tests, verifying holidays from their first year of observance (e.g., 1853 for Finland) through future projections, rather than using shorter sliding windows.
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
Learnt from: PPsyrius
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:48-50
Timestamp: 2025-06-14T11:05:21.250Z
Learning: In the holidays library, newer implementations use `start_year` to indicate the earliest year with complete holiday data coverage, not necessarily the first year a holiday existed. If a holiday system starts partway through a year (like Nauru's Public Holidays Act starting Jan 31, 1968), the start_year should be set to the following year (1969) to ensure users get full annual holiday coverage.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.504Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
Learnt from: PPsyrius
PR: vacanza/holidays#2489
File: holidays/countries/sao_tome_and_principe.py:22-26
Timestamp: 2025-04-23T14:55:35.504Z
Learning: References in holidays classes should only be included if they're used for test case cross-checks or provide historical context about when holidays were added/removed, not just for the sake of having more references.
📚 Learning: 2025-05-06T21:07:11.577Z
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: tests/countries/test_andorra.py:23-28
Timestamp: 2025-05-06T21:07:11.577Z
Learning: In the holidays project, test methods for country holidays follow a standard form where year ranges are explicitly recreated in each test method rather than being stored as class variables, to maintain consistency across all country tests.
Applied to files:
tests/countries/test_canada.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/countries/test_canada.py
📚 Learning: 2025-06-14T10:58:43.636Z
Learnt from: PPsyrius
PR: vacanza/holidays#2629
File: tests/countries/test_namibia.py:22-23
Timestamp: 2025-06-14T10:58:43.636Z
Learning: In the vacanza/holidays project, country test files consistently use range(start_year, 2050) which intentionally excludes 2050 and stops at 2049. This is a library-wide implementation pattern, not an off-by-one error.
Applied to files:
tests/countries/test_canada.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:
tests/countries/test_canada.py
📚 Learning: 2025-07-02T18:17:53.342Z
Learnt from: KJhellico
PR: vacanza/holidays#2608
File: tests/countries/test_saint_vincent_and_the_grenadines.py:162-178
Timestamp: 2025-07-02T18:17:53.342Z
Learning: In the Saint Vincent and the Grenadines holidays implementation, New Year's Day is added without observed rules using `_add_new_years_day()` and should not include observed rule testing in its test method. Only holidays explicitly wrapped with `_add_observed()` have observed behavior.
Applied to files:
tests/countries/test_canada.py
📚 Learning: 2025-05-09T18:36:09.607Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: tests/countries/test_finland.py:23-26
Timestamp: 2025-05-09T18:36:09.607Z
Learning: The holidays project prioritizes complete historical coverage in tests, verifying holidays from their first year of observance (e.g., 1853 for Finland) through future projections, rather than using shorter sliding windows.
Applied to files:
tests/countries/test_canada.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/countries/test_canada.py
📚 Learning: 2025-07-09T21:16:35.145Z
Learnt from: KJhellico
PR: vacanza/holidays#2623
File: tests/countries/test_christmas_island.py:136-146
Timestamp: 2025-07-09T21:16:35.145Z
Learning: In Christmas Island's holiday implementation, the test_christmas_day method cannot use assertNoNonObservedHoliday because in some years observed Christmas Day overlaps with Boxing Day when both holidays are moved due to weekend conflicts, causing the standard non-observed holiday check to fail inappropriately.
Applied to files:
tests/countries/test_canada.py
📚 Learning: 2025-06-16T11:42:28.293Z
Learnt from: PPsyrius
PR: vacanza/holidays#2638
File: holidays/countries/svalbard_and_jan_mayen.py:1-12
Timestamp: 2025-06-16T11:42:28.293Z
Learning: In the holidays library codebase, country implementation files in holidays/countries/ follow a standard convention of NOT having module-level docstrings. They start with the license header comment block, followed by imports, then class definitions. This is consistent across all country implementations like austria.py, belgium.py, canada.py, etc.
Applied to files:
holidays/countries/canada.py
📚 Learning: 2025-06-18T10:07:58.780Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/french_southern_territories.py:41-44
Timestamp: 2025-06-18T10:07:58.780Z
Learning: Territorial holiday classes that inherit from parent countries (like HolidaysAX from Finland, HolidaysSJ from Norway, HolidaysTF from France) follow a standard pattern of silently overriding self.subdiv in their _populate_public_holidays() method without validation, as this ensures they always use the correct subdivision code for their territory regardless of user input.
Applied to files:
holidays/countries/canada.py
📚 Learning: 2025-03-30T20:18:46.006Z
Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:24-26
Timestamp: 2025-03-30T20:18:46.006Z
Learning: In the holidays library, country classes do not directly implement `_populate()`. Instead, they implement specialized methods like `_populate_public_holidays()`, and the base class `HolidayBase` handles the orchestration by calling these specialized methods.
Applied to files:
holidays/countries/canada.py
📚 Learning: 2025-06-14T11:04:31.180Z
Learnt from: PPsyrius
PR: vacanza/holidays#2609
File: holidays/countries/nauru.py:57-60
Timestamp: 2025-06-14T11:04:31.180Z
Learning: In the holidays library, the base `HolidayBase._populate()` method already includes a guard clause that prevents holiday population methods like `_populate_public_holidays()` from being called when the year is before `start_year` or after `end_year`. Therefore, individual country implementations do not need to add their own guard clauses for years before independence or other start dates.
Applied to files:
holidays/countries/canada.py
📚 Learning: 2025-06-18T10:58:28.058Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/france.py:184-206
Timestamp: 2025-06-18T10:58:28.058Z
Learning: In the holidays library France implementation, creating shared private methods between subdivision-specific holiday population methods (like _populate_subdiv_57_public_holidays and _populate_subdiv_6ae_public_holidays) is not supported by the current framework architecture, so duplicate code between functionally identical subdivision methods should be left as-is.
Applied to files:
holidays/countries/canada.py
📚 Learning: 2025-06-18T10:26:50.180Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/france.py:178-180
Timestamp: 2025-06-18T10:26:50.180Z
Learning: In the France holidays implementation, the _populate_subdiv_57_public_holidays() (Moselle) and _populate_subdiv_6ae_public_holidays() (Alsace) methods are functionally identical, both adding Good Friday (≥1893) and Saint Stephen's Day (≥1892) with the same legal references from August 16th, 1892. Therefore, for the deprecated "Alsace-Moselle" subdivision, calling either method produces the same result.
Applied to files:
holidays/countries/canada.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/countries/canada.py
🧬 Code Graph Analysis (1)
tests/countries/test_canada.py (1)
tests/common.py (2)
assertHolidayName
(195-199)assertNoHolidayName
(273-275)
⏰ 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/countries/canada.py (1)
374-377
: NT National Aboriginal Day correctly gated from 2001
Code in_populate_subdiv_nt_public_holidays
usesif self._year >= 2001
andtest_canada.py
asserts NT holidays only from 2001 onward. The lone>= 1996
check is for Nova Scotia’s Natal Day, not Aboriginal Day. All tests and snapshots align—no further changes needed.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🇨🇦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Proposed change
Update Canada holidays: adjust introduction year of National Aboriginal Day in NT (2001).
Type of change
holidays
functionality in general)Checklist
make check
locally; all checks and tests passed.