Update Serbia holidays: set 2007 as start_year
#2939
Merged
+5
−727
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
WalkthroughIntroduces a start_year = 2007 attribute in the Serbia holiday class and inserts blank lines for readability. Updates the RS_COMMON.json snapshot by removing historical entries and retaining only three dates in 2007 (Jan 1, Jan 2, Jan 7). No other logic or API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ 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/countries/serbia.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 Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #2939 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 296 296
Lines 17596 17597 +1
Branches 2270 2270
=========================================
+ Hits 17596 17597 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
start_yearstart_year
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/serbia.py(2 hunks)snapshots/countries/RS_COMMON.json(0 hunks)
💤 Files with no reviewable changes (1)
- snapshots/countries/RS_COMMON.json
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_bosnia_and_herzegovina.py:21-24
Timestamp: 2025-09-10T16:17:30.428Z
Learning: Bosnia and Herzegovina holidays implementation currently lacks a start_year attribute. In tests/countries/test_bosnia_and_herzegovina.py, using cls.full_range = range(2000, 2050) is intentional to bound assertions and setup. Do not suggest replacing it with harness default full_range until start_year is introduced.
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#2881
File: tests/countries/test_south_africa.py:22-22
Timestamp: 2025-09-14T04:41:10.130Z
Learning: South Africa's observed holiday system only started in 1995, so in tests/countries/test_south_africa.py, using years_non_observed=range(1995, 2050) is intentional to limit testing to years where observed holidays actually exist, improving both correctness and performance.
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_bulgaria.py:21-23
Timestamp: 2025-09-14T16:14:44.954Z
Learning: For Bulgaria in the holidays library, using years_non_observed=range(2017, 2030) in tests/countries/test_bulgaria.py is intentional because all observed holiday examples in the test data fall within this range (latest being 2029), making this range appropriate for limiting testing to years where observed holidays actually exist, improving both correctness and performance.
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_bulgaria.py:21-23
Timestamp: 2025-09-14T16:14:44.954Z
Learning: For Bulgaria in the holidays library, using years_non_observed=range(2017, 2030) in tests/countries/test_bulgaria.py is intentional because all observed holiday examples in the test data fall within this range (latest being 2029), making this range appropriate for limiting testing to years where observed holidays actually exist.
Learnt from: PPsyrius
PR: vacanza/holidays#2407
File: snapshots/countries/TL_COMMON.json:7-7
Timestamp: 2025-04-03T05:59:57.480Z
Learning: In the holidays project, snapshot files (like snapshots/countries/TL_COMMON.json) are auto-generated when running `make snapshot` and should not be manually edited. Semicolons (;) in holiday entries are used as separators when multiple holidays occur on the same date.
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_barbados.py:21-23
Timestamp: 2025-09-14T16:03:13.496Z
Learning: In tests/countries/test_barbados.py, using years_non_observed=range(2000, 2024) is intentional because all observed holiday examples fall within 2001-2023, making this range appropriate for limiting testing to years where observed holidays actually exist in the test data.
Learnt from: PPsyrius
PR: vacanza/holidays#2642
File: holidays/countries/france.py:300-319
Timestamp: 2025-06-18T10:21:01.376Z
Learning: In the France holidays implementation, legislative years for holiday changes should be hard-coded rather than extracted into constants, as this maintains consistency with the existing codebase pattern and provides historical accuracy for specific legislative acts.
📚 Learning: 2025-09-10T16:17:30.428Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_bosnia_and_herzegovina.py:21-24
Timestamp: 2025-09-10T16:17:30.428Z
Learning: Bosnia and Herzegovina holidays implementation currently lacks a start_year attribute. In tests/countries/test_bosnia_and_herzegovina.py, using cls.full_range = range(2000, 2050) is intentional to bound assertions and setup. Do not suggest replacing it with harness default full_range until start_year is introduced.
Applied to files:
holidays/countries/serbia.py
📚 Learning: 2025-06-14T11:05:21.250Z
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.
Applied to files:
holidays/countries/serbia.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/serbia.py
📚 Learning: 2025-09-14T04:41:10.130Z
Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_south_africa.py:22-22
Timestamp: 2025-09-14T04:41:10.130Z
Learning: South Africa's observed holiday system only started in 1995, so in tests/countries/test_south_africa.py, using years_non_observed=range(1995, 2050) is intentional to limit testing to years where observed holidays actually exist, improving both correctness and performance.
Applied to files:
holidays/countries/serbia.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/countries/serbia.py
📚 Learning: 2025-07-08T10:21:37.055Z
Learnt from: KJhellico
PR: vacanza/holidays#2701
File: holidays/countries/palestine.py:122-122
Timestamp: 2025-07-08T10:21:37.055Z
Learning: In Palestine, Easter follows the same naming pattern as Christmas - both the first and second days of Easter have identical names for the respective religious groups (Catholic or Orthodox). The implementation correctly uses `_add_easter_sunday` for nationwide holidays and `_add_easter_monday` for group-specific second days, but both share the same Arabic name "عيد الفصح المجيد".
Applied to files:
holidays/countries/serbia.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 DO use tr() wrappers around holiday names when calling _add_* methods. This is the correct pattern for l10n-enabled country implementations, contrary to previous learning about translation being handled internally by _add_* methods.
Applied to files:
holidays/countries/serbia.py
🧬 Code graph analysis (1)
holidays/countries/serbia.py (1)
holidays/groups/christian.py (2)
_add_holy_saturday(355-362)_add_easter_sunday(286-295)
🔇 Additional comments (1)
holidays/countries/serbia.py (1)
67-78: Whitespace-only around Easter block — OKFormatting tweak only; no behavior change.
arkid15r
approved these changes
Sep 17, 2025
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
Set 2007 as
start_yearfor Serbia holidays as its independence was restored on June 5th, 2006.Any Yugoslavian-era holidays should instead be handled by its own entity
Yugoslavia/YUG/YUandSerbia and Montenegro/CS/CSG- if we ever plan to support those anyway.This is part of the non-test case changes decoupling from #2881
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.