Add Solomon Islands holidays #2632
Conversation
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughSupport for Solomon Islands holidays was added. This included updates to documentation, new country-specific holiday definitions and logic, registration of the country in the registry, corresponding import statements, and a full suite of unit tests covering national and subdivision holidays. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (14)
🧰 Additional context used🧠 Learnings (11)📓 Common learningssnapshots/countries/SB_WE.json (1)snapshots/countries/SB_TE.json (1)snapshots/countries/SB_CH.json (1)snapshots/countries/SB_RB.json (1)snapshots/countries/SB_CE.json (1)snapshots/countries/SB_IS.json (1)snapshots/countries/SB_COMMON.json (1)snapshots/countries/SB_MK.json (1)snapshots/countries/SB_ML.json (1)snapshots/countries/SB_CT.json (1)🧬 Code Graph Analysis (1)holidays/countries/__init__.py (1)
🔇 Additional comments (15)
✨ 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. 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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2632 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 234 235 +1
Lines 14806 14869 +63
Branches 2060 2063 +3
=========================================
+ Hits 14806 14869 +63 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
holidays/countries/solomon_islands.py (1)
170-187:⚠️ Potential issueIncomplete line at the end of file.
Line 187 appears to be truncated or incomplete. Consider removing it or completing the definition if something is missing.
- special_gu_public_holidays = {2020: (NOV, 18, "Public Holiday")} - + special_gu_public_holidays = {2020: (NOV, 18, "Public Holiday")}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 174-174: Line too long (162/100)
(C0301)
[convention] 175-175: Line too long (164/100)
(C0301)
[convention] 176-176: Line too long (172/100)
(C0301)
[refactor] 170-170: Too few public methods (0/2)
(R0903)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)holidays/countries/__init__.py(1 hunks)holidays/countries/solomon_islands.py(1 hunks)holidays/registry.py(1 hunks)tests/countries/test_solomon_islands.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holidays/countries/__init__.py (1)
holidays/countries/solomon_islands.py (3)
SolomonIslands(23-159)SB(162-163)SLB(166-167)
🪛 Pylint (3.3.7)
tests/countries/test_solomon_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSolomonIslands.setUpClass' method
(W0221)
[convention] 33-33: Missing function or method docstring
(C0116)
[convention] 36-36: Missing function or method docstring
(C0116)
[convention] 39-39: Missing function or method docstring
(C0116)
[convention] 51-51: Missing function or method docstring
(C0116)
[convention] 62-62: Missing function or method docstring
(C0116)
[convention] 76-76: Missing function or method docstring
(C0116)
[convention] 90-90: Missing function or method docstring
(C0116)
[convention] 104-104: Missing function or method docstring
(C0116)
[convention] 118-118: Missing function or method docstring
(C0116)
[convention] 145-145: Missing function or method docstring
(C0116)
[convention] 156-156: Missing function or method docstring
(C0116)
[convention] 169-169: Missing function or method docstring
(C0116)
[convention] 198-198: Missing function or method docstring
(C0116)
[convention] 207-207: Missing function or method docstring
(C0116)
[convention] 214-214: Missing function or method docstring
(C0116)
[convention] 221-221: Missing function or method docstring
(C0116)
[convention] 228-228: Missing function or method docstring
(C0116)
[convention] 235-235: Missing function or method docstring
(C0116)
[convention] 242-242: Missing function or method docstring
(C0116)
[convention] 249-249: Missing function or method docstring
(C0116)
[convention] 256-256: Missing function or method docstring
(C0116)
[convention] 263-263: Missing function or method docstring
(C0116)
[refactor] 20-20: Too many public methods (23/20)
(R0904)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 40-40: Line too long (173/100)
(C0301)
[convention] 41-41: Line too long (175/100)
(C0301)
[convention] 47-47: Line too long (147/100)
(C0301)
[convention] 48-48: Line too long (122/100)
(C0301)
[convention] 174-174: Line too long (162/100)
(C0301)
[convention] 175-175: Line too long (164/100)
(C0301)
[convention] 176-176: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 162-162: Missing class docstring
(C0115)
[convention] 166-166: Missing class docstring
(C0115)
[refactor] 170-170: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: Test docs build
- GitHub Check: Build distribution
🔇 Additional comments (7)
holidays/countries/solomon_islands.py (3)
23-87: Well-structured holiday class implementation.The class properly inherits from all necessary base classes and defines subdivisions comprehensively. The documentation with official references is thorough.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 40-40: Line too long (173/100)
(C0301)
[convention] 41-41: Line too long (175/100)
(C0301)
[convention] 47-47: Line too long (147/100)
(C0301)
[convention] 48-48: Line too long (122/100)
(C0301)
88-133: Holiday implementation correctly handles royal transition and observance rules.The Queen's/King's Birthday logic properly accounts for the 2023 transition and documented exceptions. Different observance rules for Christmas holidays follow the official gazette patterns.
134-160: Clean implementation of province-specific holidays.The dictionary-based approach for province days is efficient and maintainable.
holidays/registry.py (1)
178-178: Registry entry correctly added.The Solomon Islands entry follows the standard format and maintains alphabetical ordering.
holidays/countries/__init__.py (1)
175-175: Import statement properly added.The import follows the standard pattern and maintains alphabetical ordering.
README.md (1)
108-108: Documentation properly updated.The country count is correctly incremented and the Solomon Islands entry includes all subdivisions with accurate information.
Also applies to: 1235-1240
tests/countries/test_solomon_islands.py (1)
20-276: Comprehensive test coverage for Solomon Islands holidays.The tests thoroughly cover all holiday types, observance rules, and province-specific holidays with both positive and negative test cases.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSolomonIslands.setUpClass' method
(W0221)
[convention] 33-33: Missing function or method docstring
(C0116)
[convention] 36-36: Missing function or method docstring
(C0116)
[convention] 39-39: Missing function or method docstring
(C0116)
[convention] 51-51: Missing function or method docstring
(C0116)
[convention] 62-62: Missing function or method docstring
(C0116)
[convention] 76-76: Missing function or method docstring
(C0116)
[convention] 90-90: Missing function or method docstring
(C0116)
[convention] 104-104: Missing function or method docstring
(C0116)
[convention] 118-118: Missing function or method docstring
(C0116)
[convention] 145-145: Missing function or method docstring
(C0116)
[convention] 156-156: Missing function or method docstring
(C0116)
[convention] 169-169: Missing function or method docstring
(C0116)
[convention] 198-198: Missing function or method docstring
(C0116)
[convention] 207-207: Missing function or method docstring
(C0116)
[convention] 214-214: Missing function or method docstring
(C0116)
[convention] 221-221: Missing function or method docstring
(C0116)
[convention] 228-228: Missing function or method docstring
(C0116)
[convention] 235-235: Missing function or method docstring
(C0116)
[convention] 242-242: Missing function or method docstring
(C0116)
[convention] 249-249: Missing function or method docstring
(C0116)
[convention] 256-256: Missing function or method docstring
(C0116)
[convention] 263-263: Missing function or method docstring
(C0116)
[refactor] 20-20: Too many public methods (23/20)
(R0904)
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Kriti Birda <164247895+kritibirda26@users.noreply.github.com>
There was a problem hiding this comment.
I took a look at CanadaTests to try fix this but it seems to be defining subdiv_holidays in a similar way without mypy complaining there. How do I fix this error properly?
This seems to work with mypy, but I'm not so sure with doing them that way - I think the current # type: ignore looks better 😔 - what do you think? @KJhellico @arkid15r
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
holidays/countries/solomon_islands.py (1)
170-188: 🛠️ Refactor suggestionStatic-holiday dicts look good but lack subdivision routing
special_ch_public_holidays&special_gu_public_holidayswill be ignored unlessStaticHolidaysis patched to route onsubdiv. That support exists upstream only for keys named<subdiv>_special_public_holidays.
Rename the dicts accordingly to ensure they load:- special_ch_public_holidays = {2020: (NOV, 18, name)} - special_gu_public_holidays = {2020: (NOV, 18, name)} + ch_special_public_holidays = {2020: (NOV, 18, name)} + gu_special_public_holidays = {2020: (NOV, 18, name)}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 174-174: Line too long (162/100)
(C0301)
[convention] 175-175: Line too long (164/100)
(C0301)
[convention] 176-176: Line too long (172/100)
(C0301)
[refactor] 170-170: Too few public methods (0/2)
(R0903)
🪛 GitHub Actions: CI/CD
[error] 175-175: Ruff format hook failed: file was reformatted by the hook. Run 'ruff --fix' or 'pre-commit run --all-files' locally to fix formatting issues.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
README.md(2 hunks)holidays/countries/solomon_islands.py(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 40-40: Line too long (173/100)
(C0301)
[convention] 41-41: Line too long (175/100)
(C0301)
[convention] 47-47: Line too long (147/100)
(C0301)
[convention] 48-48: Line too long (122/100)
(C0301)
[convention] 174-174: Line too long (162/100)
(C0301)
[convention] 175-175: Line too long (164/100)
(C0301)
[convention] 176-176: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 162-162: Missing class docstring
(C0115)
[convention] 166-166: Missing class docstring
(C0115)
[refactor] 170-170: Too few public methods (0/2)
(R0903)
🪛 GitHub Actions: CI/CD
holidays/countries/solomon_islands.py
[error] 175-175: Ruff format hook failed: file was reformatted by the hook. Run 'ruff --fix' or 'pre-commit run --all-files' locally to fix formatting issues.
🔇 Additional comments (3)
README.md (2)
108-117: Country count hard-coded—consider deriving it automaticallyThe total country count stating “189” will drift the next time a country is added (or removed) unless someone remembers to update the number again. It’s easy to forget and CI didn’t catch it here.
A tiny helper in the docs-build step (e.g. parsing the registry and injecting the value) keeps the README self-healing and avoids future off-by-one edits.Would you like a quick snippet that counts the entries in
holidays/registry.pyand replaces the figure during the docs build?🧰 Tools
🪛 LanguageTool
[typographical] ~110-~110: Consider adding a comma after this introductory phrase.
Context: ...e same used for domain names, and for a subdivision its [ISO 3166-2 code](https://en.wikipe...(AS_A_NN_COMMA)
[uncategorized] ~114-~114: You might be missing the article “the” here.
Context: ...utput. A default language is defined bydefault_language(optional) attribute f...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
1235-1240: 👍 Row inserted in correct alphabetical orderSolomon Islands is slotted between Slovenia and South Africa, as expected. Subdivision codes match those in the implementation file.
holidays/countries/solomon_islands.py (1)
90-91: New Year’s Day: Saturday case silently ignoredYou override the default rule and pass
rule=SUN_TO_NEXT_MON, so if 1 Jan falls on a Saturday the prior-Friday shift defined inobserved_ruleis lost. Was that intentional?
If not, drop the explicitruleso the combined SAT/SUN rule from__init__applies:- self._add_observed(self._add_new_years_day("New Year's Day"), rule=SUN_TO_NEXT_MON) + self._add_observed(self._add_new_years_day("New Year's Day"))
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Kriti Birda <164247895+kritibirda26@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
holidays/countries/solomon_islands.py (2)
29-48: CI still fails on line-length / formattingSeveral lines exceed 100 chars, and the module docstring is missing – the same issues flagged earlier.
Runruff format(orpre-commit run --all-files) to satisfy the style gate before the pipeline runs again.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 40-40: Line too long (173/100)
(C0301)
[convention] 41-41: Line too long (175/100)
(C0301)
[convention] 47-47: Line too long (147/100)
(C0301)
[convention] 48-48: Line too long (122/100)
(C0301)
111-118: 🛠️ Refactor suggestionDuplicate Saturday + Friday entry for Sovereign’s Birthday
_add_observed(self._add_holiday_2nd_sat_of_jun(name))keeps the original Saturday holiday and then adds an “(observed)” Friday, so two public-holiday keys are produced for pre-2023 years. The gazette lists only the Friday.- self._add_observed(self._add_holiday_2nd_sat_of_jun(name)) + # Move the Saturday holiday to the previous Friday and drop the original. + self._move_holiday(self._add_holiday_2nd_sat_of_jun(name))
_move_holiday()removes the Saturday entry, leaving a single Friday record with the observed label.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/solomon_islands.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
holidays/countries/solomon_islands.py (5)
holidays/groups/christian.py (6)
_add_good_friday(308-317)_add_holy_saturday(319-326)_add_easter_monday(259-268)_add_whit_monday(432-441)_add_christmas_day(208-216)_add_christmas_day_two(218-226)holidays/groups/international.py (1)
_add_new_years_day(126-134)holidays/groups/custom.py (1)
StaticHolidays(18-47)holidays/observed_holiday_base.py (1)
ObservedHolidayBase(101-243)holidays/holiday_base.py (1)
_add_holiday(819-831)
🪛 Pylint (3.3.7)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 40-40: Line too long (173/100)
(C0301)
[convention] 41-41: Line too long (175/100)
(C0301)
[convention] 47-47: Line too long (147/100)
(C0301)
[convention] 48-48: Line too long (122/100)
(C0301)
[convention] 174-174: Line too long (162/100)
(C0301)
[convention] 175-175: Line too long (164/100)
(C0301)
[convention] 176-176: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 162-162: Missing class docstring
(C0115)
[convention] 166-166: Missing class docstring
(C0115)
[refactor] 170-170: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: Test docs build
- GitHub Check: Build distribution
🔇 Additional comments (2)
holidays/countries/solomon_islands.py (2)
128-132: Observed-rule asymmetry for Christmas & Boxing DayUsing
SAT_SUN_TO_NEXT_MON_TUEmeans a Saturday Christmas moves forward to Monday, whereas other fixed-date holidays (e.g. Independence Day) move back to Friday. Confirm this mirrors gazette practice; if not, adopt the default combined rule.
85-91:❓ Verification inconclusive
Check New Year’s Day Saturday handling
Here you override the rule with
SUN_TO_NEXT_MON, so 1 Jan 2028 (a Saturday) would not shift to Friday, even though the Act’s Saturday-to-Friday convention is applied elsewhere. Was this intentional?
🏁 Script executed:
#!/bin/bash # Show how New Year's Day is stored for 2028 python - <<'PY' import holidays, datetime as dt print(holidays.country_holidays("SB", years=[2028])[dt.date(2028, 1, 1):dt.date(2028, 1, 3)]) PYLength of output: 38
Review New Year’s Day observed logic
The New Year’s Day override only applies
SUN_TO_NEXT_MON, so 2028-01-01 (Saturday) doesn’t shift to Friday. The output for 2028 shows:{datetime.date(2028, 1, 1)}Should we include
SAT_TO_PREV_FRIhere as well?• File: holidays/countries/solomon_islands.py (lines 85–91)
• Current snippet:kwargs.setdefault("observed_rule", SUN_TO_NEXT_MON + SAT_TO_PREV_FRI) … self._add_observed( self._add_new_years_day("New Year's Day"), rule=SUN_TO_NEXT_MON )
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Kriti Birda <164247895+kritibirda26@users.noreply.github.com>
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 (2)
holidays/countries/solomon_islands.py(1 hunks)tests/countries/test_solomon_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/solomon_islands.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:137-156
Timestamp: 2025-06-15T08:29:43.208Z
Learning: In Solomon Islands holidays implementation, Capital Territory (CT/Honiara) does not observe any provincial day holiday, including Guadalcanal's Province Day, despite being geographically located on Guadalcanal island. CT was split from Guadalcanal Province in 1983 to become a separate self-governing territory and does not have or observe provincial day holidays.
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.
🧬 Code Graph Analysis (2)
tests/countries/test_solomon_islands.py (2)
tests/common.py (9)
CommonCountryTests(356-374)assertAliases(121-130)assertNoHolidays(292-294)assertHolidayName(195-199)assertNoHoliday(244-246)assertNoNonObservedHoliday(248-250)assertNoNonObservedHolidayName(277-279)assertNoHolidayName(273-275)assertHolidays(228-230)holidays/countries/solomon_islands.py (3)
SolomonIslands(23-167)SB(170-171)SLB(174-175)
holidays/countries/solomon_islands.py (5)
holidays/groups/christian.py (6)
_add_good_friday(308-317)_add_holy_saturday(319-326)_add_easter_monday(259-268)_add_whit_monday(432-441)_add_christmas_day(208-216)_add_christmas_day_two(218-226)holidays/groups/international.py (1)
_add_new_years_day(126-134)holidays/groups/custom.py (1)
StaticHolidays(18-47)holidays/observed_holiday_base.py (1)
ObservedHolidayBase(101-243)holidays/holiday_base.py (1)
_add_holiday(819-831)
🪛 Pylint (3.3.7)
tests/countries/test_solomon_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSolomonIslands.setUpClass' method
(W0221)
[convention] 34-34: Missing function or method docstring
(C0116)
[convention] 37-37: Missing function or method docstring
(C0116)
[convention] 40-40: Missing function or method docstring
(C0116)
[convention] 52-52: Missing function or method docstring
(C0116)
[convention] 63-63: Missing function or method docstring
(C0116)
[convention] 77-77: Missing function or method docstring
(C0116)
[convention] 91-91: Missing function or method docstring
(C0116)
[convention] 105-105: Missing function or method docstring
(C0116)
[convention] 119-119: Missing function or method docstring
(C0116)
[convention] 146-146: Missing function or method docstring
(C0116)
[convention] 157-157: Missing function or method docstring
(C0116)
[convention] 170-170: Missing function or method docstring
(C0116)
[convention] 201-201: Missing function or method docstring
(C0116)
[convention] 211-211: Missing function or method docstring
(C0116)
[convention] 218-218: Missing function or method docstring
(C0116)
[convention] 225-225: Missing function or method docstring
(C0116)
[convention] 232-232: Missing function or method docstring
(C0116)
[convention] 239-239: Missing function or method docstring
(C0116)
[convention] 246-246: Missing function or method docstring
(C0116)
[convention] 253-253: Missing function or method docstring
(C0116)
[convention] 260-260: Missing function or method docstring
(C0116)
[convention] 267-267: Missing function or method docstring
(C0116)
[refactor] 20-20: Too many public methods (23/20)
(R0904)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 30-30: Line too long (128/100)
(C0301)
[convention] 31-31: Line too long (158/100)
(C0301)
[convention] 32-32: Line too long (153/100)
(C0301)
[convention] 45-45: Line too long (165/100)
(C0301)
[convention] 46-46: Line too long (167/100)
(C0301)
[convention] 182-182: Line too long (162/100)
(C0301)
[convention] 183-183: Line too long (164/100)
(C0301)
[convention] 184-184: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 170-170: Missing class docstring
(C0115)
[convention] 174-174: Missing class docstring
(C0115)
[refactor] 178-178: Too few public methods (0/2)
(R0903)
🪛 GitHub Actions: CI/CD
tests/countries/test_solomon_islands.py
[error] 216-216: Test failure in TestSolomonIslands.test_choiseul_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 223-223: Test failure in TestSolomonIslands.test_guadalcanal_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 230-230: Test failure in TestSolomonIslands.test_isabel_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 237-237: Test failure in TestSolomonIslands.test_makira_ulawa_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 244-244: Test failure in TestSolomonIslands.test_malaita_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 251-251: Test failure in TestSolomonIslands.test_rennell_and_bellona_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 258-258: Test failure in TestSolomonIslands.test_temotu_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
[error] 265-265: Test failure in TestSolomonIslands.test_western_province_day due to dateutil.parser.ParserError: Unknown string format with invalid date string containing a list instead of a single date.
🔇 Additional comments (2)
holidays/countries/solomon_islands.py (2)
86-92: Default observed rule wired in correctly – nice touch
setdefault("observed_rule", SUN_TO_NEXT_MON + SAT_TO_PREV_FRI)makes the instance honour both Sunday-to-Monday and Saturday-to-Friday shifts while still allowing a caller to override. Clean and future-proof.
109-126: Birthday logic reads clearly and handles the outliersThe ternary for the King/Queen label + the
sovereign_birthday_datesoverride table together cover the rule-change (2023→) and the 2022 / 2025 exceptions. Implementation mirrors the gazette exactly—LGTM.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/countries/test_solomon_islands.py (1)
191-199: 🛠️ Refactor suggestionAvoid in-loop mutation of
name– it breaks subsequent assertions
nameis reassigned to its “ (observed)” variant while theforloop is still iterating.
All iterations after the matching subdivision now see the mutated value, so theelse
branch no longer checks for the original holiday name in the remaining subdivisions
(coverage gap + potential false positives).- name = f"{name} (observed)" - self.assertHolidayName(name, holidays, observed_dts) + observed_name = f"{name} (observed)" + self.assertHolidayName(observed_name, holidays, observed_dts) ... - self.assertNoHolidayName(name, holidays, range(1979, 2050)) + self.assertNoHolidayName(name, holidays, range(1979, 2050)) + self.assertNoHolidayName(observed_name, holidays, range(1979, 2050))Same issue was raised earlier but remains unfixed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/solomon_islands.py(1 hunks)tests/countries/test_solomon_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
holidays/countries/solomon_islands.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:137-156
Timestamp: 2025-06-15T08:29:43.208Z
Learning: In Solomon Islands holidays implementation, Capital Territory (CT/Honiara) does not observe any provincial day holiday, including Guadalcanal's Province Day, despite being geographically located on Guadalcanal island. CT was split from Guadalcanal Province in 1983 to become a separate self-governing territory and does not have or observe provincial day holidays.
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.
🪛 Pylint (3.3.7)
tests/countries/test_solomon_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSolomonIslands.setUpClass' method
(W0221)
[convention] 34-34: Missing function or method docstring
(C0116)
[convention] 37-37: Missing function or method docstring
(C0116)
[convention] 40-40: Missing function or method docstring
(C0116)
[convention] 52-52: Missing function or method docstring
(C0116)
[convention] 63-63: Missing function or method docstring
(C0116)
[convention] 77-77: Missing function or method docstring
(C0116)
[convention] 91-91: Missing function or method docstring
(C0116)
[convention] 105-105: Missing function or method docstring
(C0116)
[convention] 119-119: Missing function or method docstring
(C0116)
[convention] 146-146: Missing function or method docstring
(C0116)
[convention] 157-157: Missing function or method docstring
(C0116)
[convention] 170-170: Missing function or method docstring
(C0116)
[convention] 201-201: Missing function or method docstring
(C0116)
[convention] 211-211: Missing function or method docstring
(C0116)
[convention] 215-215: Missing function or method docstring
(C0116)
[convention] 219-219: Missing function or method docstring
(C0116)
[convention] 223-223: Missing function or method docstring
(C0116)
[convention] 227-227: Missing function or method docstring
(C0116)
[convention] 231-231: Missing function or method docstring
(C0116)
[convention] 237-237: Missing function or method docstring
(C0116)
[convention] 241-241: Missing function or method docstring
(C0116)
[convention] 245-245: Missing function or method docstring
(C0116)
[refactor] 20-20: Too many public methods (23/20)
(R0904)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 30-30: Line too long (128/100)
(C0301)
[convention] 31-31: Line too long (158/100)
(C0301)
[convention] 32-32: Line too long (153/100)
(C0301)
[convention] 45-45: Line too long (165/100)
(C0301)
[convention] 46-46: Line too long (167/100)
(C0301)
[convention] 191-191: Line too long (162/100)
(C0301)
[convention] 192-192: Line too long (164/100)
(C0301)
[convention] 193-193: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 179-179: Missing class docstring
(C0115)
[convention] 183-183: Missing class docstring
(C0115)
[refactor] 187-187: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: Build distribution
- GitHub Check: Test docs build
🔇 Additional comments (2)
holidays/countries/solomon_islands.py (2)
90-96: Confirm compositeobserved_ruleis still honoured for New Year’s Day
_add_observed(..., rule=SUN_TO_NEXT_MON)intentionally suppresses
the default Saturday-to-Friday shift you configured in
__init__.
If that is policy (only Sundays observed for New Year), perfect.
If Saturdays should also shift, drop the explicitrule=argument so
the combinedSUN_TO_NEXT_MON + SAT_TO_PREV_FRIapplies.No change required if the single-rule behaviour is correct.
116-125: Edge cases covered – nice!The bespoke mapping for 2022 & 2025 plus the fallback logic cleanly
handles the Sovereign’s Birthday transition.
Implementation is clear and matches gazette data.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
tests/countries/test_solomon_islands.py (2)
208-210: Also assert absence of the observed label for other provincesThe helper currently checks that the base holiday name is absent in unrelated subdivisions, but it doesn’t check the
" (observed)"variant. A quick addition tightens coverage.- self.assertNoHolidayName(holiday_name, holidays, range(1979, 2050)) + self.assertNoHolidayName(holiday_name, holidays, range(1979, 2050)) + self.assertNoHolidayName( + f"{holiday_name} (observed)", holidays, range(1979, 2050) + )
196-200: Zero-pad generated dates to keep"YYYY-MM-DD"formatWithout padding, the generator yields strings like
1979-6-29, while the rest of the suite (anddateutil) expect1979-06-29. This leads to inconsistent assertions and brittle parsing.- holiday_name, holidays, (f"{year}-{month}-{day}" for year in range(1979, 2050)) + holiday_name, + holidays, + ( + f"{year}-{month:02d}-{day:02d}" + for year in range(1979, 2050) + ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/solomon_islands.py(1 hunks)tests/countries/test_solomon_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:90-97
Timestamp: 2025-06-15T20:39:51.064Z
Learning: Solomon Islands Saturday → Friday holiday shift rule was systematically implemented only from around 2021-2022 onward, not consistently applied since 1979. The Public Holidays Act only explicitly mentions Sunday → Monday shifts, with Saturday shifts handled through discretionary ministerial gazette notices.
tests/countries/test_solomon_islands.py (1)
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.
holidays/countries/solomon_islands.py (6)
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:90-97
Timestamp: 2025-06-15T20:39:51.064Z
Learning: Solomon Islands Saturday → Friday holiday shift rule was systematically implemented only from around 2021-2022 onward, not consistently applied since 1979. The Public Holidays Act only explicitly mentions Sunday → Monday shifts, with Saturday shifts handled through discretionary ministerial gazette notices.
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:137-156
Timestamp: 2025-06-15T08:29:43.208Z
Learning: In Solomon Islands holidays implementation, Capital Territory (CT/Honiara) does not observe any provincial day holiday, including Guadalcanal's Province Day, despite being geographically located on Guadalcanal island. CT was split from Guadalcanal Province in 1983 to become a separate self-governing territory and does not have or observe provincial day holidays.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:28-54
Timestamp: 2025-06-15T15:10:20.335Z
Learning: In the vacanza/holidays project, ruff configuration allows longer docstring lines than the typical 100-character limit, so docstring reference lines that exceed standard line length limits are acceptable and won't be flagged by ruff format during pre-commit checks.
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:28-54
Timestamp: 2025-06-15T15:10:20.335Z
Learning: The vacanza/holidays project uses ruff with line-length = 99 characters (configured in pyproject.toml), not the standard 80 or 100 character limits. When pre-commit checks pass, the formatting is compliant with the project's specific ruff configuration.
Learnt from: PPsyrius
PR: vacanza/holidays#2637
File: holidays/countries/singapore.py:49-50
Timestamp: 2025-06-16T11:17:34.653Z
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.
🪛 Pylint (3.3.7)
tests/countries/test_solomon_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSolomonIslands.setUpClass' method
(W0221)
[convention] 34-34: Missing function or method docstring
(C0116)
[convention] 37-37: Missing function or method docstring
(C0116)
[convention] 40-40: Missing function or method docstring
(C0116)
[convention] 55-55: Missing function or method docstring
(C0116)
[convention] 68-68: Missing function or method docstring
(C0116)
[convention] 82-82: Missing function or method docstring
(C0116)
[convention] 96-96: Missing function or method docstring
(C0116)
[convention] 110-110: Missing function or method docstring
(C0116)
[convention] 124-124: Missing function or method docstring
(C0116)
[convention] 151-151: Missing function or method docstring
(C0116)
[convention] 162-162: Missing function or method docstring
(C0116)
[convention] 175-175: Missing function or method docstring
(C0116)
[refactor] 188-188: Too many arguments (6/5)
(R0913)
[refactor] 188-188: Too many positional arguments (6/5)
(R0917)
[convention] 211-211: Missing function or method docstring
(C0116)
[convention] 221-221: Missing function or method docstring
(C0116)
[convention] 231-231: Missing function or method docstring
(C0116)
[convention] 241-241: Missing function or method docstring
(C0116)
[convention] 251-251: Missing function or method docstring
(C0116)
[convention] 261-261: Missing function or method docstring
(C0116)
[convention] 271-271: Missing function or method docstring
(C0116)
[convention] 283-283: Missing function or method docstring
(C0116)
[convention] 293-293: Missing function or method docstring
(C0116)
[convention] 303-303: Missing function or method docstring
(C0116)
[refactor] 20-20: Too many public methods (23/20)
(R0904)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 30-30: Line too long (128/100)
(C0301)
[convention] 31-31: Line too long (158/100)
(C0301)
[convention] 32-32: Line too long (153/100)
(C0301)
[convention] 45-45: Line too long (165/100)
(C0301)
[convention] 46-46: Line too long (167/100)
(C0301)
[convention] 193-193: Line too long (162/100)
(C0301)
[convention] 194-194: Line too long (164/100)
(C0301)
[convention] 195-195: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 181-181: Missing class docstring
(C0115)
[convention] 185-185: Missing class docstring
(C0115)
[refactor] 189-189: Too few public methods (0/2)
(R0903)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (1)
holidays/countries/solomon_islands.py (1)
34-53: ```shell
#!/bin/bashLocate the SolomonIslands holiday definition
FILE=$(rg --files-with-matches "class SolomonIslands" -l)
echo "File found: $FILE"Show top of file to inspect docstring and class signature
sed -n '1,100p' "$FILE"
Find all usages of the Saturday→Friday rule
echo -e "\nInstances of SAT_TO_PREV_FRI:"
grep -n "SAT_TO_PREV_FRI" "$FILE" || echo "None found"Check for any observed_since constraints
echo -e "\nInstances of observed_since:"
grep -n "observed_since" "$FILE" || echo "None found"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: ~Jhellico <KJhellico@users.noreply.github.com> Signed-off-by: Kriti Birda <164247895+kritibirda26@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
holidays/countries/solomon_islands.py (1)
97-98: Duplicate of earlier feedback: unconditionalSUN_TO_NEXT_MON + SAT_TO_PREV_FRImay over-apply to pre-2016 data.
See previous discussion for potential conditionalisation.tests/countries/test_solomon_islands.py (1)
191-203: Zero-pad month/day in generated date strings for consistency
f"{year}-{month}-{day}"yields variants like1980-6-2.
Everywhere else in the test-suite we use the canonicalYYYY-MM-DDformat, so the mix looks untidy (though it still parses).- holiday_name, holidays, (f"{year}-{month}-{day}" for year in range(1979, 2050)) + holiday_name, + holidays, + ( + f"{year}-{month:02d}-{day:02d}" + for year in range(1979, 2050) + ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
holidays/countries/solomon_islands.py(1 hunks)tests/countries/test_solomon_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:90-97
Timestamp: 2025-06-15T20:39:51.064Z
Learning: Solomon Islands Saturday → Friday holiday shift rule was systematically implemented only from around 2021-2022 onward, not consistently applied since 1979. The Public Holidays Act only explicitly mentions Sunday → Monday shifts, with Saturday shifts handled through discretionary ministerial gazette notices.
holidays/countries/solomon_islands.py (7)
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:90-97
Timestamp: 2025-06-15T20:39:51.064Z
Learning: Solomon Islands Saturday → Friday holiday shift rule was systematically implemented only from around 2021-2022 onward, not consistently applied since 1979. The Public Holidays Act only explicitly mentions Sunday → Monday shifts, with Saturday shifts handled through discretionary ministerial gazette notices.
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:137-156
Timestamp: 2025-06-15T08:29:43.208Z
Learning: In Solomon Islands holidays implementation, Capital Territory (CT/Honiara) does not observe any provincial day holiday, including Guadalcanal's Province Day, despite being geographically located on Guadalcanal island. CT was split from Guadalcanal Province in 1983 to become a separate self-governing territory and does not have or observe provincial day holidays.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:28-54
Timestamp: 2025-06-15T15:10:20.335Z
Learning: In the vacanza/holidays project, ruff configuration allows longer docstring lines than the typical 100-character limit, so docstring reference lines that exceed standard line length limits are acceptable and won't be flagged by ruff format during pre-commit checks.
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:28-54
Timestamp: 2025-06-15T15:10:20.335Z
Learning: The vacanza/holidays project uses ruff with line-length = 99 characters (configured in pyproject.toml), not the standard 80 or 100 character limits. When pre-commit checks pass, the formatting is compliant with the project's specific ruff configuration.
Learnt from: PPsyrius
PR: vacanza/holidays#2637
File: holidays/countries/singapore.py:49-50
Timestamp: 2025-06-16T11:17:34.653Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.624Z
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.
🧬 Code Graph Analysis (1)
tests/countries/test_solomon_islands.py (2)
tests/common.py (8)
TestCase(28-338)CommonCountryTests(356-374)assertAliases(121-130)assertNoHolidays(292-294)assertHolidayName(195-199)assertNoHoliday(244-246)assertNoNonObservedHoliday(248-250)assertHolidays(228-230)holidays/countries/solomon_islands.py (3)
SolomonIslands(23-187)SB(190-191)SLB(194-195)
🪛 Pylint (3.3.7)
tests/countries/test_solomon_islands.py
[convention] 1-1: Missing module docstring
(C0114)
[convention] 20-20: Missing class docstring
(C0115)
[warning] 22-22: Number of parameters was 4 in 'TestCase.setUpClass' and is now 1 in overriding 'TestSolomonIslands.setUpClass' method
(W0221)
[convention] 34-34: Missing function or method docstring
(C0116)
[convention] 37-37: Missing function or method docstring
(C0116)
[convention] 40-40: Missing function or method docstring
(C0116)
[convention] 55-55: Missing function or method docstring
(C0116)
[convention] 68-68: Missing function or method docstring
(C0116)
[convention] 82-82: Missing function or method docstring
(C0116)
[convention] 96-96: Missing function or method docstring
(C0116)
[convention] 110-110: Missing function or method docstring
(C0116)
[convention] 124-124: Missing function or method docstring
(C0116)
[convention] 154-154: Missing function or method docstring
(C0116)
[convention] 165-165: Missing function or method docstring
(C0116)
[convention] 178-178: Missing function or method docstring
(C0116)
[refactor] 191-191: Too many arguments (6/5)
(R0913)
[refactor] 191-191: Too many positional arguments (6/5)
(R0917)
[convention] 214-214: Missing function or method docstring
(C0116)
[convention] 224-224: Missing function or method docstring
(C0116)
[convention] 234-234: Missing function or method docstring
(C0116)
[convention] 244-244: Missing function or method docstring
(C0116)
[convention] 254-254: Missing function or method docstring
(C0116)
[convention] 264-264: Missing function or method docstring
(C0116)
[convention] 274-274: Missing function or method docstring
(C0116)
[convention] 286-286: Missing function or method docstring
(C0116)
[convention] 296-296: Missing function or method docstring
(C0116)
[convention] 306-306: Missing function or method docstring
(C0116)
[refactor] 20-20: Too many public methods (23/20)
(R0904)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 30-30: Line too long (128/100)
(C0301)
[convention] 31-31: Line too long (130/100)
(C0301)
[convention] 32-32: Line too long (163/100)
(C0301)
[convention] 33-33: Line too long (154/100)
(C0301)
[convention] 34-34: Line too long (158/100)
(C0301)
[convention] 35-35: Line too long (152/100)
(C0301)
[convention] 36-36: Line too long (153/100)
(C0301)
[convention] 49-49: Line too long (165/100)
(C0301)
[convention] 53-53: Line too long (167/100)
(C0301)
[convention] 202-202: Line too long (162/100)
(C0301)
[convention] 203-203: Line too long (164/100)
(C0301)
[convention] 204-204: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 190-190: Missing class docstring
(C0115)
[convention] 194-194: Missing class docstring
(C0115)
[refactor] 198-198: Too few public methods (0/2)
(R0903)
🔇 Additional comments (3)
holidays/countries/solomon_islands.py (1)
133-134: Let’s inspect the dynamic helper generator inholidays/holiday_base.py:#!/bin/bash rg -n "def __getattr__" -A50 holidays/holiday_base.pytests/countries/test_solomon_islands.py (2)
214-223: Historic Friday-shift assertions may overreach – please verify sourcesThe test expects Saturday-to-Friday shifts for Central Province Day as far back as 2013/2014.
Per Public Holidays Act only Sunday→Monday is automatic; Saturday shifts have been ad-hoc (see ministerial gazettes) and appear systematic only from ~2021-22 (see project learning note).If the underlying rules in
holidays/countries/solomon_islands.pywere not updated to hard-code those pre-2021 shifts, these assertions will fail silently when data is refreshed.Can you double-check the primary sources for 2013/2014 and either:
- Remove the early Friday dates, or
- Extend the holiday class with the appropriate special-case logic.
24-32:setUpClasssignature differs fromunittest.TestCase– double-check linter noiseOverloading
setUpClass(cls)with extra positional arguments triggers PylintW0221.
The call‐site inCommonCountryTests.setUpClass()still works, so it’s harmless functionally, but the warning will keep coming back.If you want to silence it without sacrificing readability, consider:
- @classmethod - def setUpClass(cls): + @classmethod # noqa: D401 + def setUpClass(cls) -> None: # type: ignore[override]or disable the warning in the project config.
Feel free to ignore if your tooling already whitelists this pattern.⛔ Skipped due to learnings
Learnt from: KJhellico PR: vacanza/holidays#2609 File: tests/countries/test_nauru.py:20-24 Timestamp: 2025-06-13T15:15:25.128Z Learning: In the vacanza/holidays test suite, overriding `setUpClass` is intentionally done with the single `cls` parameter (no *args/**kwargs), so signature-mismatch lint warnings are ignored project-wide.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
holidays/countries/solomon_islands.py (2)
97-98: Observed-rule still unconditional – see earlier feedbackHard-coding both
SUN_TO_NEXT_MON + SAT_TO_PREV_FRIfor every year ignores the historical nuance we already discussed.
If the goal is fidelity pre-2016, gateSAT_TO_PREV_FRIbehind the year check; otherwise please mark that limitation in the docstring.
104-105: Palau-style carry-over caveat still undocumentedThe
self._next_year_new_years_daypattern means the 31 Dec observed date only materialises when the caller requests both Y-1 and Y.
Users pulling just{Y-1}will miss it.Either add a short note in the class docstring or move the helper to a shared mix-in so every country solves this once.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/countries/solomon_islands.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: KJhellico
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:90-97
Timestamp: 2025-06-15T20:39:51.064Z
Learning: Solomon Islands Saturday → Friday holiday shift rule was systematically implemented only from around 2021-2022 onward, not consistently applied since 1979. The Public Holidays Act only explicitly mentions Sunday → Monday shifts, with Saturday shifts handled through discretionary ministerial gazette notices.
holidays/countries/solomon_islands.py (7)
<retrieved_learning>
Learnt from: KJhellico
PR: #2632
File: holidays/countries/solomon_islands.py:90-97
Timestamp: 2025-06-15T20:39:51.064Z
Learning: Solomon Islands Saturday → Friday holiday shift rule was systematically implemented only from around 2021-2022 onward, not consistently applied since 1979. The Public Holidays Act only explicitly mentions Sunday → Monday shifts, with Saturday shifts handled through discretionary ministerial gazette notices.
</retrieved_learning>
<retrieved_learning>
Learnt from: PPsyrius
PR: #2632
File: holidays/countries/solomon_islands.py:137-156
Timestamp: 2025-06-15T08:29:43.208Z
Learning: In Solomon Islands holidays implementation, Capital Territory (CT/Honiara) does not observe any provincial day holiday, including Guadalcanal's Province Day, despite being geographically located on Guadalcanal island. CT was split from Guadalcanal Province in 1983 to become a separate self-governing territory and does not have or observe provincial day holidays.
</retrieved_learning>
<retrieved_learning>
Learnt from: KJhellico
PR: #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.
</retrieved_learning>
<retrieved_learning>
Learnt from: KJhellico
PR: #2632
File: holidays/countries/solomon_islands.py:28-54
Timestamp: 2025-06-15T15:10:20.335Z
Learning: In the vacanza/holidays project, ruff configuration allows longer docstring lines than the typical 100-character limit, so docstring reference lines that exceed standard line length limits are acceptable and won't be flagged by ruff format during pre-commit checks.
</retrieved_learning>
<retrieved_learning>
Learnt from: KJhellico
PR: #2632
File: holidays/countries/solomon_islands.py:28-54
Timestamp: 2025-06-15T15:10:20.335Z
Learning: The vacanza/holidays project uses ruff with line-length = 99 characters (configured in pyproject.toml), not the standard 80 or 100 character limits. When pre-commit checks pass, the formatting is compliant with the project's specific ruff configuration.
</retrieved_learning>
<retrieved_learning>
Learnt from: PPsyrius
PR: #2637
File: holidays/countries/singapore.py:49-50
Timestamp: 2025-06-16T11:17:34.653Z
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.
</retrieved_learning>
<retrieved_learning>
Learnt from: PPsyrius
PR: #2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.624Z
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.
</retrieved_learning>
🪛 Pylint (3.3.7)
holidays/countries/solomon_islands.py
[convention] 29-29: Line too long (134/100)
(C0301)
[convention] 30-30: Line too long (128/100)
(C0301)
[convention] 31-31: Line too long (130/100)
(C0301)
[convention] 32-32: Line too long (163/100)
(C0301)
[convention] 33-33: Line too long (154/100)
(C0301)
[convention] 34-34: Line too long (158/100)
(C0301)
[convention] 35-35: Line too long (152/100)
(C0301)
[convention] 36-36: Line too long (153/100)
(C0301)
[convention] 49-49: Line too long (165/100)
(C0301)
[convention] 53-53: Line too long (167/100)
(C0301)
[convention] 202-202: Line too long (162/100)
(C0301)
[convention] 203-203: Line too long (164/100)
(C0301)
[convention] 204-204: Line too long (172/100)
(C0301)
[convention] 1-1: Missing module docstring
(C0114)
[convention] 190-190: Missing class docstring
(C0115)
[convention] 194-194: Missing class docstring
(C0115)
[refactor] 198-198: Too few public methods (0/2)
(R0903)
# Conflicts: # README.md
|
Proposed change
Add Solomon Islands holidays.
Resolves #1269.
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green