-
-
Notifications
You must be signed in to change notification settings - Fork 542
Update Singapore holidays: add 2026 public holidays #2637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Summary by CodeRabbit
WalkthroughThe changes update Singapore's 2026 public holiday dates across the holiday calculation logic, snapshots, and tests. Confirmed holiday dates replace estimated ones, observed days are added, and a new test verifies the 2026 holiday set. Documentation is also updated with a new reference URL. Changes
Assessment against linked issues
Suggested labels
✨ 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 #2637 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 232 232
Lines 14739 14739
Branches 2057 2057
=========================================
Hits 14739 14739 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Considering the number of false positive we get from SonarCloud's Code Analysis - is there a way to somewhat raise the threshold for code duplication? @arkid15r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
holidays/countries/singapore.py
(6 hunks)snapshots/countries/SG_COMMON.json
(1 hunks)tests/countries/test_singapore.py
(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_singapore.py (2)
Learnt from: PPsyrius
PR: vacanza/holidays#2386
File: tests/countries/test_nepal.py:499-536
Timestamp: 2025-04-05T06:49:06.217Z
Learning: In the holidays project, test files follow a dual testing approach: individual methods test specific holidays across multiple years, while comprehensive year-specific tests (e.g., `test_2025`) verify all holidays for a specific year in a single assertion. Both approaches serve different testing purposes and complement each other.
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.
🧬 Code Graph Analysis (1)
tests/countries/test_singapore.py (1)
tests/common.py (1)
assertHolidays
(228-230)
🪛 Pylint (3.3.7)
holidays/countries/singapore.py
[convention] 49-49: Line too long (132/100)
(C0301)
tests/countries/test_singapore.py
[convention] 197-197: Missing function or method docstring
(C0116)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (2)
holidays/countries/singapore.py (1)
180-306
: Let’s pull down the PDF and extract text before matching dates – grepping a PDF stream won’t work. Trying a quick Python + pdfminer approach:#!/bin/bash set -e # Download the official 2026 public-holidays PDF url='https://www.mom.gov.sg/-/media/mom/documents/employment-practices/public-holidays/public-holidays-2026.pdf' curl -sL "$url" -o public-holidays-2026.pdf # Install a PDF‐to‐text extractor pip install --quiet pdfminer.six # Extract and list all “<day> <Month> 2026” occurrences python3 - <<'EOF' import re from pdfminer.high_level import extract_text text = extract_text('public-holidays-2026.pdf') dates = sorted(set(re.findall(r'(\d{1,2} [A-Z][a-z]+ 2026)', text))) print('\n'.join(dates)) EOFsnapshots/countries/SG_COMMON.json (1)
963-975
: Snapshot perfectly mirrors code & testsConfirmed that the 2026 snapshot lines match the constants and
test_2026
; good job keeping everything in sync.
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
I'll check what can be done there. Thanks for complaining 👍 |
Proposed change
Add 2026 Public Holidays [1] [2] for Singapore.
Resolves #2636 .
Type of change
holidays
functionality in general)Checklist
make check
, all checks and tests are green