-
-
Notifications
You must be signed in to change notification settings - Fork 554
Add no holiday countries #2969
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
Add no holiday countries #2969
Conversation
Summary by CodeRabbit
WalkthroughAdds three uninhabited-territory providers (Bouvet Island BV/BVT, British Indian Ocean Territory IO/IOT, Heard Island and McDonald Islands HM/HMD) as NoHoliday providers: new country modules, exports and registry entries, a NoHolidayBase, README update, and unit tests confirming aliases and no holidays. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (8)
🧰 Additional context used🧠 Learnings (21)📚 Learning: 2025-07-12T20:54:28.749Z
Applied to files:
📚 Learning: 2025-04-05T04:29:38.042Z
Applied to files:
📚 Learning: 2025-09-18T03:19:23.722Z
Applied to files:
📚 Learning: 2025-05-06T21:07:11.577Z
Applied to files:
📚 Learning: 2025-04-05T04:47:27.213Z
Applied to files:
📚 Learning: 2025-09-20T12:21:50.877Z
Applied to files:
📚 Learning: 2025-04-05T04:50:40.752Z
Applied to files:
📚 Learning: 2025-08-08T14:37:03.045Z
Applied to files:
📚 Learning: 2025-09-14T06:39:32.896Z
Applied to files:
📚 Learning: 2025-09-10T16:17:30.428Z
Applied to files:
📚 Learning: 2025-09-25T08:56:22.473Z
Applied to files:
📚 Learning: 2025-09-02T08:02:03.604Z
Applied to files:
📚 Learning: 2025-09-17T09:07:56.459Z
Applied to files:
📚 Learning: 2025-09-14T16:03:13.558Z
Applied to files:
📚 Learning: 2025-09-17T09:07:56.459Z
Applied to files:
📚 Learning: 2025-09-03T14:05:10.592Z
Applied to files:
📚 Learning: 2025-09-14T16:05:55.205Z
Applied to files:
📚 Learning: 2025-06-14T11:04:31.180Z
Applied to files:
📚 Learning: 2025-03-30T20:18:46.006Z
Applied to files:
📚 Learning: 2025-04-25T20:27:59.086Z
Applied to files:
📚 Learning: 2025-08-31T19:34:20.731Z
Applied to files:
🧬 Code graph analysis (7)holidays/countries/bouvet_island.py (1)
tests/countries/test_bouvet_island.py (4)
holidays/countries/british_indian_ocean_territory.py (1)
holidays/no_holiday_base.py (1)
tests/countries/test_british_indian_ocean_territory.py (2)
tests/countries/test_heard_island_and_mcdonald_islands.py (2)
holidays/countries/heard_island_and_mcdonald_islands.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). (2)
🔇 Additional comments (7)
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.13.3)holidays/countries/bouvet_island.py�[1;31mruff failed�[0m holidays/countries/british_indian_ocean_territory.py�[1;31mruff failed�[0m tests/countries/test_british_indian_ocean_territory.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 #2969 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 301 305 +4
Lines 17968 17996 +28
Branches 2321 2321
=========================================
+ Hits 17968 17996 +28 ☔ 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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
README.md
(3 hunks)holidays/no_holiday_base.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_belgium.py:28-30
Timestamp: 2025-09-20T12:21:50.877Z
Learning: Belgium holidays implementation currently lacks a start_year attribute. In tests/countries/test_belgium.py, do not suggest adding test_no_holidays methods that rely on start_year until the start_year attribute is introduced to Belgium's holiday implementation.
Learnt from: KJhellico
PR: vacanza/holidays#2820
File: holidays/countries/__init__.py:199-203
Timestamp: 2025-08-31T19:34:20.731Z
Learning: In Saint Helena, Ascension and Tristan da Cunha holidays implementation (holidays/countries/saint_helena_ascension_and_tristan_da_cunha.py), only three classes are defined: SaintHelenaAscensionAndTristanDaCunha (main class), SH (alias), and SHN (alias). There is no HolidaysSH class, despite it being referenced in the registry.py entry.
Learnt from: KJhellico
PR: vacanza/holidays#2774
File: tests/countries/test_liberia.py:15-16
Timestamp: 2025-08-08T14:37:03.045Z
Learning: When adding a new country in vacanza/holidays, also re-export it in holidays/countries/__init__.py (e.g., from .liberia import Liberia, LR, LBR) so tests and users can import from holidays.countries consistently.
Learnt from: KJhellico
PR: vacanza/holidays#2820
File: holidays/countries/__init__.py:199-203
Timestamp: 2025-08-31T19:34:20.731Z
Learning: In the Saint Helena, Ascension and Tristan da Cunha implementation, there's a discrepancy between the registry.py entry (which references "HolidaysSH") and the actual implementation (which only defines SaintHelenaAscensionAndTristanDaCunha, SH, SHN, and SaintHelenaAscensionAndTristanDaCunhaStaticHolidays classes). The HolidaysSH class doesn't exist and according to the maintainer isn't needed.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.641Z
Learning: Library-wide holiday patterns and their optimizations should be handled at the base class level (like InternationalHolidays) rather than documenting workarounds in individual country modules. This maintains separation of concerns and avoids documentation duplication.
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#2881
File: tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py:209-209
Timestamp: 2025-09-14T17:17:14.387Z
Learning: In tests/countries/test_saint_helena_ascension_and_tristan_da_cunha.py, the explicit loop iteration pattern for subdivision-specific holiday checks (like Anniversary Day for TA subdivision) is intentionally preferred over using assertSubdivTa helper methods, as confirmed by PPsyrius.
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_indonesia.py:221-223
Timestamp: 2025-09-28T05:42:12.755Z
Learning: In tests/countries/test_indonesia.py, the manual set inclusion checks using get_named and years_found for Lunar New Year (test_lunar_new_year) and Vesak Day (test_vesak_day) are intentional and should remain until both holidays get their own `{insert}_no_estimated` flags implemented, rather than using standard harness assertions like assertHolidayName/assertNoHolidayName.
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.
📚 Learning: 2025-06-21T16:30:12.749Z
Learnt from: KJhellico
PR: vacanza/holidays#2654
File: holidays/countries/cape_verde.py:1-12
Timestamp: 2025-06-21T16:30:12.749Z
Learning: The holidays project does not use module docstrings in country holiday files. All country files start directly with the copyright header comment block without module docstrings, maintaining a consistent coding style across the project.
Applied to files:
holidays/no_holiday_base.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/no_holiday_base.py
📚 Learning: 2025-06-22T21:33:17.854Z
Learnt from: KJhellico
PR: vacanza/holidays#2671
File: holidays/countries/libya.py:1-1
Timestamp: 2025-06-22T21:33:17.854Z
Learning: In the holidays library, country files do not use module docstrings. The established pattern is: license header comment block, imports, then class definitions with class docstrings. Module docstrings should not be added to country files as they don't follow this convention.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-06-01T17:58:53.279Z
Learnt from: KJhellico
PR: vacanza/holidays#2583
File: holidays/countries/niger.py:1-1
Timestamp: 2025-06-01T17:58:53.279Z
Learning: In the holidays project, module-level docstrings are not required or needed for country holiday files in the holidays/countries/ directory.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-06-16T15:48:48.680Z
Learnt from: PPsyrius
PR: vacanza/holidays#2615
File: tests/countries/test_anguilla.py:1-12
Timestamp: 2025-06-16T15:48:48.680Z
Learning: Test files in the holidays repository follow a standardized structure without module or class docstrings. All country test files use the same pattern: license header, imports, and class definition (`class Test{Country}(CommonCountryTests, TestCase):`) without docstrings. This is an established codebase convention that should be maintained for consistency.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-06-19T05:54:49.792Z
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/french_polynesia.py:1-12
Timestamp: 2025-06-19T05:54:49.792Z
Learning: The holidays library uses a standard file header format across all country implementation files consisting of a comprehensive comment block with project description, authors, website, and license information. Country files do not use module-level docstrings but instead rely on this header format followed by class-level docstrings.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-06-10T12:43:10.577Z
Learnt from: ankushhKapoor
PR: vacanza/holidays#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.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-04-26T19:55:09.581Z
Learnt from: KJhellico
PR: vacanza/holidays#2501
File: scripts/docs/gen_index.py:1-12
Timestamp: 2025-04-26T19:55:09.581Z
Learning: The holidays project uses a standard header for all Python (.py) files in the repository that describes the overall holidays library, including authors, website and license information, regardless of the specific purpose of individual script files.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-06-15T15:24:53.055Z
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: holidays/countries/faroe_islands.py:62-67
Timestamp: 2025-06-15T15:24:53.055Z
Learning: The `HolidayBase` class uses `__getattr__` to dynamically implement `_add_holiday_*` methods through pattern matching, including patterns like `_add_holiday_<n>_days_past_easter`, `_add_holiday_<month>_<day>`, and various weekday-relative patterns. Methods like `_add_holiday_26_days_past_easter` are not explicitly defined but are dynamically generated when called.
Applied to files:
holidays/no_holiday_base.py
📚 Learning: 2025-07-12T20:54:28.749Z
Learnt from: KJhellico
PR: vacanza/holidays#2719
File: README.md:108-108
Timestamp: 2025-07-12T20:54:28.749Z
Learning: In PR #2719 adding Faroe Islands, KJhellico confirmed that the country count in README.md should be updated from 213 to 214, making 214 the accurate total after this addition.
Applied to files:
README.md
📚 Learning: 2025-06-06T16:02:09.910Z
Learnt from: KJhellico
PR: vacanza/holidays#2606
File: README.md:562-568
Timestamp: 2025-06-06T16:02:09.910Z
Learning: The README.md country table displays ISO 3166-1 alpha-2 codes only in the "Code" column. Alpha-3 codes or country aliases should not be included in this table format, maintaining consistency with all other country entries.
Applied to files:
README.md
🧬 Code graph analysis (1)
holidays/no_holiday_base.py (1)
holidays/holiday_base.py (1)
HolidayBase
(57-1297)
⏰ 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 (4)
README.md (4)
108-120
: Count bump looks good.Nice to see the total refreshed to 248 right alongside the new providers.
360-365
: Bouvet Island row lands perfectly.Placement, formatting, and blank fields line up with the established pattern.
374-379
: British Indian Ocean Territory entry reads clean.Everything mirrors the no-holiday template—no tweaks needed.
829-834
: Heard Island and McDonald Islands entry checks out.Alphabetical slotting and empty fields look right on target.
Co-Authored-By: ~Jhellico <KJhellico@users.noreply.github.com>
|
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.
PTAL
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.
LGTM.
Proposed change
This is a PoC for 3 no holiday entities we need to cover for ISO 3166-1 compliance:
I started it trying to optimize it by overriding
HolidayBase::__init__
but then switched to keep it simpler approach and left as is for now. Any suggestions are welcome.Resolves #2425 .
Resolves #2661 .
Resolves #2663 .
Type of change
holidays
functionality in general)Checklist
make check
locally; all checks and tests passed.