Skip to content

Conversation

@PPsyrius
Copy link
Collaborator

Proposed change

Remove duplicate holidays label assignment for Singapore as done elsewhere library-wide i.e.

    "2000-02-07": "Chinese New Year (estimated) (observed)",
    "2000-02-07": "Chinese New Year (observed, estimated)",

Type of change

  • New country/market holidays support (thank you!)
  • Supported country/market holidays update (calendar discrepancy fix, localization)
  • Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
  • Dependency update (version deprecation/pin/upgrade)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (a code change causing existing functionality to break)
  • New feature (new holidays functionality in general)

Checklist

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 21, 2025

Summary by CodeRabbit

  • New Features
    • Added “estimated” and “observed, estimated” labels for Singapore holidays to improve clarity where dates are approximate.
  • Bug Fixes
    • Standardized label order to “observed, estimated” across Singapore holiday entries.
    • Corrected SG holiday data; removed an incorrect Eid al-Fitr occurrence on 2030-02-04.
  • Documentation
    • Updated and added translations for new labels: English (US/SG) entries added; Thai translations provided (including “observed”).
  • Chores
    • Refreshed locale metadata (version, revision dates, generator) for SG translation files.

Walkthrough

Adds two label attributes to Singapore holidays. Updates en_SG, en_US, and th SG.po metadata and adds new “estimated” and “observed, estimated” strings (with Thai translations). Standardizes SG snapshot labels to “observed, estimated” and removes one Eid al-Fitr entry on 2030-02-04.

Changes

Cohort / File(s) Summary of changes
Country class labels
holidays/countries/singapore.py
Added public attributes: estimated_label and observed_estimated_label using tr(). No logic changes.
Locale updates
holidays/locale/en_SG/.../SG.po, holidays/locale/en_US/.../SG.po, holidays/locale/th/.../SG.po
Updated headers; added "%s (estimated)" and "%s (observed, estimated)". Thai adds translations; en_SG entries added with empty msgstr; en_US msgstr mirrors msgid.
Snapshot data standardization
snapshots/countries/SG_COMMON.json
Standardized labels to “(observed, estimated)” across entries; adjusted multiple holiday names accordingly; removed Eid al-Fitr on 2030-02-04.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

snapshot

Suggested reviewers

  • KJhellico

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5203f05 and 7ae3713.

📒 Files selected for processing (5)
  • holidays/countries/singapore.py (1 hunks)
  • holidays/locale/en_SG/LC_MESSAGES/SG.po (1 hunks)
  • holidays/locale/en_US/LC_MESSAGES/SG.po (1 hunks)
  • holidays/locale/th/LC_MESSAGES/SG.po (1 hunks)
  • snapshots/countries/SG_COMMON.json (16 hunks)
🧰 Additional context used
🧠 Learnings (22)
📚 Learning: 2025-06-28T10:39:19.185Z
Learnt from: KJhellico
PR: vacanza/holidays#2684
File: holidays/locale/it/LC_MESSAGES/SM.po:13-13
Timestamp: 2025-06-28T10:39:19.185Z
Learning: In the holidays project, .po file header comments use the format "# [Country] holidays." for default language files (without trailing hash) and "# [Country] holidays [locale] localization." for non-default language files (also without trailing hash).

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
📚 Learning: 2025-05-06T15:25:44.333Z
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: holidays/locale/ca/LC_MESSAGES/AD.po:31-40
Timestamp: 2025-05-06T15:25:44.333Z
Learning: In the Holidays project, msgid fields in localization files contain strings in the entity's default language (as defined by default_language attribute), not English source strings as in standard gettext implementations.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
  • holidays/locale/th/LC_MESSAGES/SG.po
📚 Learning: 2025-06-26T15:34:35.476Z
Learnt from: KJhellico
PR: vacanza/holidays#2676
File: holidays/locale/en_US/LC_MESSAGES/TN.po:17-28
Timestamp: 2025-06-26T15:34:35.476Z
Learning: In the holidays project, .po file header metadata updates (version numbers, revision dates, translator information) are legitimate changes when part of localization work and don't require `make l10n` regeneration. The `make l10n` command is primarily for formatting fixes and missing translator comments, not for intentional metadata updates.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
  • holidays/locale/th/LC_MESSAGES/SG.po
📚 Learning: 2025-03-05T17:51:00.633Z
Learnt from: KJhellico
PR: vacanza/holidays#2259
File: holidays/locale/en_IN/LC_MESSAGES/IN.po:30-299
Timestamp: 2025-03-05T17:51:00.633Z
Learning: In the Holidays project, .po files for a country's default locale use empty msgstr fields as a standard convention.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
📚 Learning: 2025-03-30T18:25:07.087Z
Learnt from: KJhellico
PR: vacanza/holidays#2388
File: holidays/locale/en_CI/LC_MESSAGES/CI.po:88-88
Timestamp: 2025-03-30T18:25:07.087Z
Learning: In the holidays library, localization files have a specific structure: message comments are in standard English (en_US) describing the holiday, while actual translations (msgstr) should use the locale-specific terminology (e.g., en_CI for Ivory Coast English). For example, "Night of Power" in standard English is translated as "Lailatou-Kadr" in Ivory Coast English.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
📚 Learning: 2025-03-08T11:28:48.652Z
Learnt from: KJhellico
PR: vacanza/holidays#2259
File: holidays/locale/en_IN/LC_MESSAGES/IN.po:285-299
Timestamp: 2025-03-08T11:28:48.652Z
Learning: In the holidays project, message IDs (msgids) in locale files use region-specific naming conventions (e.g., "Muharram", "Id-ul-Fitr" in en_IN locale for India), while translator comments use internationally recognized names from the project's default locale (en_US) such as "Ashura", "Eid al-Fitr". This difference is intentional for proper localization.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
📚 Learning: 2025-06-11T18:32:25.595Z
Learnt from: ankushhKapoor
PR: vacanza/holidays#2601
File: holidays/locale/en_MN/LC_MESSAGES/MN.po:13-14
Timestamp: 2025-06-11T18:32:25.595Z
Learning: For non-default locale `.po` files, the header comment format is:
`# <Country> holidays <locale> localization.` (no trailing hash).

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
📚 Learning: 2025-06-29T09:37:35.283Z
Learnt from: KJhellico
PR: vacanza/holidays#2687
File: holidays/locale/en_US/LC_MESSAGES/CF.po:13-28
Timestamp: 2025-06-29T09:37:35.283Z
Learning: In the holidays project, .po files follow a standard formatting convention where there is always a blank line after the metadata header section (after the "X-Source-Language" line). This blank line separates the header from the actual translation content and should not be removed.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
📚 Learning: 2025-06-25T20:55:00.642Z
Learnt from: KJhellico
PR: vacanza/holidays#2651
File: holidays/locale/nl/LC_MESSAGES/BQ.po:29-95
Timestamp: 2025-06-25T20:55:00.642Z
Learning: In the holidays library, Dutch locale files (.po) with `X-Source-Language: nl` should have empty msgstr entries when the target language is also Dutch. The library uses fallback=True with gettext, which returns the original msgid when msgstr is empty. This is the correct pattern for native language files and does not cause blank holiday names.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
📚 Learning: 2025-08-19T20:04:26.215Z
Learnt from: KJhellico
PR: vacanza/holidays#2831
File: holidays/countries/south_sudan.py:28-31
Timestamp: 2025-08-19T20:04:26.215Z
Learning: In the holidays library, message comments (comments describing message format templates like "%s (estimated)." and "%s (observed, estimated).") always end with a period, even when the corresponding string values do not include the period.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
  • holidays/locale/th/LC_MESSAGES/SG.po
📚 Learning: 2025-06-25T10:09:29.029Z
Learnt from: PPsyrius
PR: vacanza/holidays#2676
File: holidays/locale/ar/LC_MESSAGES/EG.po:0-0
Timestamp: 2025-06-25T10:09:29.029Z
Learning: In the holidays library, msgstr fields can be left empty for source/default_language files when using Lingva, the localization tool used by the project.

Applied to files:

  • holidays/locale/en_US/LC_MESSAGES/SG.po
  • holidays/locale/en_SG/LC_MESSAGES/SG.po
📚 Learning: 2025-05-10T04:02:13.815Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:249-253
Timestamp: 2025-05-10T04:02:13.815Z
Learning: Holiday name comments directly above tr() function calls in the holidays package should only contain the holiday name itself (e.g., "# Independence Day.") without any additional context, dates, or historical information.

Applied to files:

  • holidays/locale/en_SG/LC_MESSAGES/SG.po
📚 Learning: 2025-06-25T10:12:30.448Z
Learnt from: PPsyrius
PR: vacanza/holidays#2676
File: holidays/countries/tunisia.py:25-25
Timestamp: 2025-06-25T10:12:30.448Z
Learning: The holidays library intentionally duplicates label strings like `estimated_label` across country modules rather than centralizing them. This duplication is the established library-wide approach and should not be suggested for refactoring.

Applied to files:

  • holidays/countries/singapore.py
📚 Learning: 2025-08-21T04:51:16.392Z
Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:13-13
Timestamp: 2025-08-21T04:51:16.392Z
Learning: In the holidays library, country classes with localization support consistently use `from gettext import gettext as tr` import and wrap class-level attributes like `estimated_label`, `observed_label`, and `observed_estimated_label` with `tr()` calls. This is the standard library-wide practice for l10n-enabled entities and is required for proper string extraction when generating .po files.

Applied to files:

  • holidays/countries/singapore.py
📚 Learning: 2025-08-21T04:56:03.711Z
Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.711Z
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/singapore.py
📚 Learning: 2025-08-21T04:56:03.711Z
Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.711Z
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/singapore.py
📚 Learning: 2025-06-26T13:20:10.172Z
Learnt from: PPsyrius
PR: vacanza/holidays#2678
File: holidays/locale/th/LC_MESSAGES/US.po:30-34
Timestamp: 2025-06-26T13:20:10.172Z
Learning: In the holidays library Thai localizations, the "observed" format consistently uses "ชดเชย%s" without spaces between the Thai word and placeholder across all .po files in the codebase.

Applied to files:

  • holidays/locale/th/LC_MESSAGES/SG.po
📚 Learning: 2025-04-03T05:59:57.480Z
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.

Applied to files:

  • snapshots/countries/SG_COMMON.json
📚 Learning: 2025-03-19T16:54:58.657Z
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.

Applied to files:

  • snapshots/countries/SG_COMMON.json
📚 Learning: 2025-03-19T16:53:00.375Z
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.

Applied to files:

  • snapshots/countries/SG_COMMON.json
📚 Learning: 2025-04-13T19:10:31.502Z
Learnt from: KJhellico
PR: vacanza/holidays#2465
File: holidays/countries/suriname.py:219-251
Timestamp: 2025-04-13T19:10:31.502Z
Learning: The `_CustomIslamicHolidays` classes in this project contain only exact verified holiday dates from reliable sources, rather than calculated or estimated future dates. This is by design to ensure accuracy, particularly for religious holidays that may follow lunar calendars or depend on local observations.

Applied to files:

  • snapshots/countries/SG_COMMON.json
📚 Learning: 2025-04-13T20:41:56.613Z
Learnt from: KJhellico
PR: vacanza/holidays#2386
File: holidays/countries/nepal.py:266-284
Timestamp: 2025-04-13T20:41:56.613Z
Learning: The Islamic holiday dates in the holidays library should only include officially verified dates, not predicted ones, to maintain accuracy. This is especially important for holidays that depend on lunar observations like Eid al-Fitr and Eid al-Adha.

Applied to files:

  • snapshots/countries/SG_COMMON.json
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Test build on windows-latest
  • GitHub Check: Test build on ubuntu-latest
  • GitHub Check: Test build on macos-latest
🔇 Additional comments (8)
holidays/locale/th/LC_MESSAGES/SG.po (2)

17-27: Header metadata updates look good.

Version bump, revision date, and generator update align with l10n conventions. Keeping X-Source-Language and the blank line after headers is consistent with project style.


30-39: Thai translations for new labels are correct and consistent.

  • "%s (estimated)" → "%s (โดยประมาณ)" is accurate.
  • "%s (observed, estimated)" → "ชดเชย%s (โดยประมาณ)" preserves the no-space pattern before %s for “observed,” per prior Thai locale guidance.
    Message comments end with periods as expected.
holidays/locale/en_US/LC_MESSAGES/SG.po (2)

17-27: Header refresh + separator blank line: OK.

Matches project formatting norms for .po headers.


30-39: New combined/estimated label strings added correctly.

Including en_US translations identical to msgid is appropriate for this locale. Placement before the existing “observed” block mirrors other locales.

holidays/locale/en_SG/LC_MESSAGES/SG.po (2)

17-27: Header metadata updates are correct for the default locale.

Comment header “Singapore holidays.” and empty msgstr convention retained. Good.


30-39: Added msgids for combined/estimated labels with empty msgstr: correct.

For en_SG (default language), leaving msgstr empty is the right pattern so gettext falls back to msgid.

holidays/countries/singapore.py (1)

63-69: Review Verified: ObservedHolidayBase Supports Combined Labels

The grep output confirms that ObservedHolidayBase checks for observed_estimated_label and applies it (lines 178–190 in observed_holiday_base.py), and there’s no concatenated “(observed) (estimated)” fallback in singapore.py. Your additions of estimated_label and observed_estimated_label at the class level are correctly recognized and will yield unified labels. Feel free to merge these changes as-is.

snapshots/countries/SG_COMMON.json (1)

608-608: All SG_COMMON.json suffixes normalized

  • No matches for (observed)(estimated) or (estimated)(observed) patterns in snapshots/countries/SG_COMMON.json
  • 22 occurrences of the normalized “(observed, estimated)” form found
  • Normalization is consistent across all entries; changes look good to merge
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (5203f05) to head (7ae3713).
⚠️ Report is 3 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #2844   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          286       286           
  Lines        17038     17040    +2     
  Branches      2229      2229           
=========================================
+ Hits         17038     17040    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@PPsyrius PPsyrius mentioned this pull request Aug 21, 2025
4 tasks
Copy link
Collaborator

@KJhellico KJhellico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Good catch 👍

@arkid15r arkid15r added this pull request to the merge queue Aug 21, 2025
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Merged via the queue into vacanza:dev with commit d17356f Aug 21, 2025
36 checks passed
@PPsyrius PPsyrius deleted the sg_remove_duplicate_label branch August 22, 2025 02:14
@arkid15r arkid15r mentioned this pull request Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants