Add Fiji holidays #2354
Add Fiji holidays #2354
Conversation
Summary by CodeRabbit
WalkthroughThis pull request introduces comprehensive support for Fiji holidays, including a new module that defines holiday data and rules specific to Fiji. It updates the country registry to include Fiji with its respective ISO codes, adds a JSON file containing holiday data from 2016 to 2050, and provides a test suite to validate the new functionality. Additionally, it modifies an existing method to handle optional date inputs more effectively. Changes
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 300000ms (1)
🔇 Additional comments (2)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
README.rst(1 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/fiji.py(1 hunks)holidays/registry.py(1 hunks)snapshots/countries/FJ_COMMON.json(1 hunks)tests/countries/test_fiji.py(1 hunks)
🔇 Additional comments (11)
README.rst (1)
444-448: Addition of Fiji looks good.The Fiji entry follows the correct alphabetical order and matches the established format with appropriate ISO code and WORKDAY category.
holidays/registry.py (1)
75-75: LGTM on Fiji registry entry.Entry follows the correct format with proper country name and ISO codes ("fiji": ("Fiji", "FJ", "FJI")).
holidays/countries/__init__.py (1)
66-66: Import statement correctly added.The import follows the consistent pattern used for other countries and is in the proper alphabetical position.
snapshots/countries/FJ_COMMON.json (1)
1-1180: Comprehensive Fiji holiday dataset looks good.The JSON file provides a thorough collection of Fiji holidays from 1970-2050, including:
- Standard holidays (New Year's, Christmas)
- Religious observances (Easter, Eid al-Fitr, Diwali)
- National holidays (Fiji Day, National Youth Day)
- Cultural observances (Ratu Sir Lala Sukuna Day, Girmit Day)
The data formatting follows project standards with consistent date formatting and proper handling of observed holidays when they fall on weekends.
tests/countries/test_fiji.py (3)
176-193: Looks good - 2023 Fiji holidays are properly testedThe test verifies all expected 2023 holidays including the newly added Girmit Day (May 14). This provides good validation of the Fiji holiday implementation.
195-212: Looks good - 2024 Fiji holidays are properly testedThe test comprehensively covers the expected 2024 holidays, providing good validation of future dates in the implementation.
214-229: Looks good - Localization tests are properly implementedThe test_l10n_default method verifies that holiday names are properly localized, providing important validation for the display of holidays to users.
holidays/countries/fiji.py (4)
26-40: Looks good - Fiji class implementation with appropriate inheritanceThe Fiji class properly inherits from relevant base classes and provides good documentation with references. The class attributes are appropriately defined for Fiji's specific requirements.
88-88: Implementation mismatch with commentThe comment indicates that November 16, 2020 should also be a day off for Diwali, but this isn't implemented in the code.
Please verify if November 16, 2020 should be included as a holiday. If so, you'll need to implement this by either:
- Adding another entry for 2020 in the implementation
- Using a different data structure to support multiple dates for the same holiday in a given year
123-128: Looks good - Country code aliases properly definedThe
FJandFJIclasses are correctly implemented as aliases for theFijiclass, following the project's pattern for supporting both 2-letter and 3-letter ISO country codes.
112-114:✅ Verification successful
Verify Ratu Sir Lala Sukuna Day historical gap
The implementation adds Ratu Sir Lala Sukuna Day for years <= 2010 or >= 2023, creating a gap between 2011 and 2022. Please verify this is historically accurate.
🌐 Web query:
Was Ratu Sir Lala Sukuna Day not observed in Fiji between 2011 and 2022?💡 Result:
Ratu Sir Lala Sukuna Day was not observed as a public holiday in Fiji between 2011 and 2022. This hiatus followed Prime Minister Voreqe Bainimarama's 2010 decision to remove it from the list of national holidays[1][7]. The holiday, which honors Fiji's influential statesman and land reform architect Ratu Sir Lala Sukuna, had been celebrated since colonial times on the last Monday of May until its cancellation[1][10].
Key details about the suspension:
- The holiday was abolished through a cabinet decision in March 2010, with the government claiming public holidays weren't essential for commemorations[1].
- Public holiday lists for 2011-2022 (including years like 2011[8][11] and 2022[16][18]) confirm its absence during this period.
- Criticism emerged over the cancellation, with some viewing it as eroding cultural heritage[4][7].
The holiday was reinstated in 2023 by Prime Minister Sitiveni Rabuka's coalition government, who called Ratu Sukuna's legacy "integral to our national story"[3][7][9]. The 2025 public holiday calendar now includes Ratu Sukuna Day on May 30[2], reflecting its restored status as a day honoring Fiji's foundational leadership and indigenous governance systems[4][10].
Citations:
- 1: https://en.wikipedia.org/wiki/Ratu_Sir_Lala_Sukuna_Day
- 2: https://www.fiji.gov.fj/About-Fiji/Public-Holidays
- 3: https://www.fiji.gov.fj/Media-Centre/Speeches/English/WHEN-RATU-SUKUNA-WEPT-BY-PM-RABUKA
- 4: https://www.fijitimes.com.fj/the-life-and-legacy-of-ratu-sir-lala-sukuna/
- 5: https://www.timeanddate.com/holidays/fiji
- 6: https://en.wikipedia.org/wiki/Fiji
- 7: https://www.officeholidays.com/holidays/fiji/ratu-sukuna-day
- 8: https://www.timeanddate.com/holidays/fiji/2011
- 9: https://www.fijivillage.com/news/Cabinet-approves-Ratu-Sir-Lala-Sukuna-Day-and-Girmit-Day-and-removes-Constitution-Day-as-a-public-holiday-f48r5x/
- 10: https://en.wikipedia.org/wiki/Lala_Sukuna
- 11: https://holidayapi.com/countries/fj/2011
- 12: https://www.parliament.gov.fj/wp-content/uploads/2023/04/2Daily-Hansard-Friday-31st-March-2023.pdf
- 13: https://www.history-pas-de-calais.com/first-world-war/they-came-from-across-the-globe/fijians/
- 14: https://www.fiji.gov.fj/Media-Centre/News/Public-holidays-and-school-dates
- 15: https://www.usp.ac.fj/news/the-historical-significance-of-ratu-sir-lala-sukuna/
- 16: https://www.fijivillage.com/news/Ten-public-holidays-for-2022-rx8f45/
- 17: https://www.unifiji.ac.fj/wp-content/uploads/2016/11/RATU-SIR-LALA-SUKUNA-MEMORIAL-LECTURE.pdf
- 18: https://truecalendar.com/public-holidays/fiji/2022
- 19: https://vocal.media/motivation/ratu-sukuna
- 20: https://bank-holidays.com/public-holidays/publicholidays_2011_62.htm
Historical Gap Verified: Ratu Sir Lala Sukuna Day is Correctly Omitted for 2011–2022
The web verification confirms that Ratu Sir Lala Sukuna Day was not observed as a public holiday between 2011 and 2022. The current implementation in
holidays/countries/fiji.py(lines 112-114) correctly reflects this historical gap by adding the holiday only for years ≤ 2010 or ≥ 2023. No additional changes are needed.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2354 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 204 205 +1
Lines 12999 13054 +55
Branches 1861 1869 +8
=========================================
+ Hits 12999 13054 +55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@Prateekshit73 I ended up implementing all the code review suggestions above as Prateekshit73#4 and some extra ones which include:
Aside from the changes I've already made, it would be nice if we could also get Fiji's actual |
Fiji code review suggestions
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/fiji.py(1 hunks)snapshots/countries/FJ_COMMON.json(1 hunks)tests/countries/test_fiji.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
tests/countries/test_fiji.py (1)
holidays/countries/fiji.py (3) (3)
Fiji(31-135)FJ(138-139)FJI(142-143)
🔇 Additional comments (4)
snapshots/countries/FJ_COMMON.json (1)
1-533: Looks consistent
All entries follow the expected key-value format and align with known Fiji holidays from 2016 to 2050.tests/countries/test_fiji.py (1)
20-319: Comprehensive coverage
These tests thoroughly verify Fiji holidays from 2016 onward, reflecting correct public and workday holiday checks.holidays/countries/fiji.py (2)
69-70: Confirm observed rule usage
Different lines use an extended observed rule, e.g.,SAT_SUN_TO_NEXT_MON_TUE. Consider simplifying or confirming consistency with local practices.
120-124: Respectful Boxing Day handling
UsingSAT_SUN_TO_NEXT_MON_TUEfor observed Boxing Day is coherent with standard holiday observation rules.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/fiji.py(1 hunks)snapshots/countries/FJ_COMMON.json(1 hunks)tests/countries/test_fiji.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/fiji.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:185-188
Timestamp: 2025-03-19T16:53:00.375Z
Learning: In the Fiji holidays implementation, the `special_public_holidays_observed` dictionary in `FijiStaticHolidays` is only used for exceptions to the normal observance rules, not for documenting all holidays. Only 2019's Constitution Day needed a special entry as it didn't follow the standard patterns.
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-19T16:54:58.657Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.
🧬 Code Definitions (1)
tests/countries/test_fiji.py (1)
holidays/countries/fiji.py (3) (3)
Fiji(31-135)FJ(138-139)FJI(142-143)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test build on windows-latest
🔇 Additional comments (8)
snapshots/countries/FJ_COMMON.json (1)
1-534: All holiday entries appear consistent and correctly formatted.This large, single-purpose JSON file is beneficial for a quick lookup of Fiji’s public holidays from 2016 to 2050, including observed and estimated variations. It aligns well with the holiday definitions in the codebase: Girmit Day, Ratu Sir Lala Sukuna Day, Constitution Day for the correct years, and relevant “(observed)” or “(estimated)” tags.
tests/countries/test_fiji.py (2)
20-26: Robust test initialization.Using a broad year range (2016–2050) and capturing workday holidays in
cls.workday_holidaysensures extensive test coverage. This is a solid foundation.
197-318: Comprehensive year-by-year holiday checks.Verifying holidays from 2019 through 2025 against official data is valuable. You may also consider spot tests for years beyond 2025 to confirm future coverage is correct, given the extended JSON file.
holidays/countries/fiji.py (5)
63-70: Initialization and observed rule look good.Assigning
SAT_SUN_TO_NEXT_MONas the default and applyingSAT_SUN_TO_NEXT_MON_TUEfor Christmas/Boxing Day is correct. This approach aligns with Fiji’s official rules.
72-125: Public holiday definitions are thorough.Each holiday is named and observed properly, including Girmit Day’s move to the nearest Monday starting in 2023. Great handling of transitional logic for Constitution Day (on or before 2022) versus later years.
146-159: Clear approach to Diwali date management.Defining explicit Diwali dates for 2016–2025 and relying on
_add_diwalifor future estimates is consistent with project guidelines. No concerns here.
185-188: Proper use ofspecial_public_holidays_observed.Limiting exceptions to only 2019’s Constitution Day is ideal, as it cleanly addresses that outlier. This aligns with the library’s approach.
126-136:❓ Verification inconclusive
Clarify shifting Constitution Day to workday category from 2023.
As written, Constitution Day remains a public holiday through 2022, then shifts to a workday holiday from 2023 onward. Verify with official references that removing it as a public holiday is intentional.
🏁 Script executed:
#!/bin/bash # Gathering official references to confirm Constitution Day's status after 2022. rg -A 5 "Constitution Day" ./holidays/countries/fiji.pyLength of output: 491
Clarification Needed: Constitution Day Holiday Status Shift
It appears that Constitution Day is maintained as a public holiday for earlier years (e.g., the 2019 mapping) but from 2023 it’s added via the workday holiday logic (_add_holiday_sep_7) in populate_workday_holidays. Could you verify with official sources whether this change—from a public holiday through 2022 to a workday holiday starting 2023—is correct? If so, a brief inline comment (or commit message update) indicating the source would help clarify the intent for future maintainers.
- Constitution Day is still referenced in the public holidays mapping (e.g., for 2019).
- Starting with 2023, it is added as a workday holiday.
- Please confirm the official stance on this shift and update the documentation accordingly.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/fiji.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/fiji.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:185-188
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the Fiji holidays implementation, the `special_public_holidays_observed` dictionary in `FijiStaticHolidays` is only used for exceptions to the normal observance rules, not for documenting all holidays. Only 2019's Constitution Day needed a special entry as it didn't follow the standard patterns.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test build on windows-latest
🔇 Additional comments (4)
holidays/countries/fiji.py (4)
1-12: File header and doc disclaimers look good.
73-85: Constructor and default observed rule look properly set.
138-142: Confirm the rationale for placing Constitution Day in workday holidays from 2023 onward.If Constitution Day is no longer recognized as a public holiday, this approach is correct. Otherwise, verify that splitting it between public and workday holidays is intentional.
152-165: Diwali date mapping for 2016–2025 looks consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
holidays/countries/fiji.py (1)
72-86: 🧹 Nitpick (assertive)Revisit observed rule
Your approach of defaulting toSAT_SUN_TO_NEXT_MONis fine. If any consecutive holidays need separate weekdays, considerSAT_SUN_TO_NEXT_MON_TUEfor them or handle them individually.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/fiji.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/fiji.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:185-188
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the Fiji holidays implementation, the `special_public_holidays_observed` dictionary in `FijiStaticHolidays` is only used for exceptions to the normal observance rules, not for documenting all holidays. Only 2019's Constitution Day needed a special entry as it didn't follow the standard patterns.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
🔇 Additional comments (4)
holidays/countries/fiji.py (4)
30-36: Straightforward class definition
Looks consistent with the library’s requirement.
88-139: Solid holiday definitions
Your logic for adding and observing holidays, including shifting ‘Girmit Day’ and excluding Constitution Day post-2022, aligns with official information.
140-145: Verify Constitution Day classification
You switched Constitution Day to a workday holiday from 2023 onward. Double-check that official sources confirm it’s strictly not recognized as a public holiday starting 2023, and consider adding a test for it.
154-167: Explicit Diwali coverage
Covering 2016–2025 with official references is a practical approach. Automatic calculation beyond 2025 ensures continued coverage. Nicely done.
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Prateekshit Jaiswal <67361301+Prateekshit73@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/fiji.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/fiji.py (3)
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:146-159
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the holidays library implementation, explicit holiday dates (like Diwali in Fiji) are only defined for historical years with official sources (2016-2025). Future dates beyond the explicitly defined range are automatically calculated by methods like `_add_diwali`, which provide approximations when official dates aren't yet available.
Learnt from: PPsyrius
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:185-188
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the Fiji holidays implementation, the `special_public_holidays_observed` dictionary in `FijiStaticHolidays` is only used for exceptions to the normal observance rules, not for documenting all holidays. Only 2019's Constitution Day needed a special entry as it didn't follow the standard patterns.
Learnt from: KJhellico
PR: vacanza/holidays#2354
File: holidays/countries/fiji.py:63-70
Timestamp: 2025-03-26T17:15:40.859Z
Learning: In the holidays library, the `SAT_SUN_TO_NEXT_MON_TUE` rule is specifically used for consecutive holidays (like Christmas Day and Boxing Day) to ensure they're observed on separate weekdays (Monday and Tuesday) when they fall on weekends, while `SAT_SUN_TO_NEXT_MON` is used as the default rule for other holidays.
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test build on windows-latest
🔇 Additional comments (8)
holidays/countries/fiji.py (8)
1-12: Good use of standardized information header.The introduction and license references are well structured, offering clear provenance.
13-27: Imports appear well organized and relevant.No apparent redundancies or missing imports. The usage of
_CustomHinduHolidaysand_CustomIslamicHolidaysaligns with the library’s design.
37-60: Concise and informative docstring.The references provide solid external sources for validation. This fosters maintainability and verifiability of holiday data.
73-87: Constructor logic is consistent with the library’s patterns.
- Setting the default observed rule to
SAT_SUN_TO_NEXT_MONis appropriate for most local holidays.- Initializing each parent class in turn is correct and follows best practices for multiple inheritance.
89-140: Public holiday population method is thorough.
- The combination of
_add_observed()calls and holiday-specific additions is correct.- Well-considered usage of
ALL_TO_NEAREST_MONfor Girmit Day.- Boxing Day and Christmas Day transitions use the specialized
SAT_SUN_TO_NEXT_MON_TUErule to avoid consecutive Monday observances.Overall, the methodology aligns with the holiday library’s approach and should cover edge cases effectively.
137-139: Solid handling of Mawlid.Iterating over the returned dates from
_add_mawlid_dayand applying_add_observedensures multi-day correctness if needed.
141-145: Workday holiday approach is consistent.The separate
_populate_workday_holidaysmethod helps maintain clarity between public and workday categories.
155-169: Comprehensive Diwali mapping.Defining explicit dates for 2016–2025 addresses historical accuracy. Future expansions can rely on the base
_add_diwalilogic if needed.
|
Proposed change
Add Fiji holidays
Closes #1180
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green