Skip to content

Conversation

KJhellico
Copy link
Collaborator

Proposed change

Update Andorra holidays, add l10n support (ca, en_US, uk).

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

Copy link
Contributor

coderabbitai bot commented May 6, 2025

Summary by CodeRabbit

  • New Features

    • Added support for Andorra holidays in Catalan (default), US English, and Ukrainian, including detailed localization for holiday names.
    • Expanded and refined holiday coverage for Andorra, including government-only holidays and subdivision-specific festivals.
    • Introduced partial-day observances (e.g., Epiphany Eve, Maundy Thursday, Christmas Eve, New Year's Eve from 1pm).
  • Bug Fixes

    • Corrected holiday names, dates, and added missing holidays for Andorra and its subdivisions.
  • Documentation

    • Updated the README to reflect new supported languages and holiday categories for Andorra.
  • Tests

    • Enhanced and expanded test coverage for Andorra holidays, including localization and subdivision-specific checks.

Summary by CodeRabbit

  • New Features

    • Added full localization support for Andorra holidays in Catalan (default), US English, and Ukrainian.
    • Expanded and refined holiday definitions for Andorra, including detailed subdivision-specific holidays and accurate historical coverage.
  • Bug Fixes

    • Corrected holiday names, dates, and removed obsolete entries (e.g., Constitution Day) for Andorra and its subdivisions.
  • Documentation

    • Updated documentation to reflect supported languages for Andorra.
  • Tests

    • Significantly expanded and improved holiday test coverage, including localization and subdivision-specific holiday validation.

Walkthrough

This update introduces comprehensive localization support for Andorra's holidays, including Catalan, US English, and Ukrainian translations. It revises holiday naming, refines subdivision-specific holidays, and adjusts holiday dates in both code and data snapshots. The test suite is expanded for detailed, localized, and year-spanning coverage.

Changes

Files/Groups Change Summary
README.md Added supported language codes and holiday categories for Andorra in the "Available Countries" table.
holidays/countries/andorra.py Added gettext-based localization for holiday names; set default language to Catalan and supported languages to Catalan, US English, and Ukrainian; defined supported categories (government, public); refined holiday logic including subdivision-specific holidays with year-based conditions; changed Carnival holiday from Tuesday to Monday; introduced government holidays with partial-day observance.
holidays/groups/christian.py Fixed docstrings for Carnival and other Christian holiday methods for accuracy and consistency; no code logic changes.
holidays/locale/ca/LC_MESSAGES/AD.po,
holidays/locale/en_US/LC_MESSAGES/AD.po,
holidays/locale/uk/LC_MESSAGES/AD.po
Added new gettext PO files providing Catalan, US English, and Ukrainian translations for Andorra holidays, enabling multi-language support.
snapshots/countries/AD_02.json,
snapshots/countries/AD_03.json,
snapshots/countries/AD_04.json,
snapshots/countries/AD_05.json,
snapshots/countries/AD_07.json,
snapshots/countries/AD_08.json,
snapshots/countries/AD_COMMON.json
Updated Andorra holiday JSON snapshots from 1950–2050: shifted Carnival dates earlier by one day; renamed "National Day" to "Our Lady of Meritxell"; renamed "Immaculate Conception Day" to "Immaculate Conception"; removed or renamed subdivision festival holidays; added government half-day observances (Epiphany Eve, Maundy Thursday, Christmas Eve, New Year's Eve); refined subdivision-specific holidays with conditional year logic; standardized holiday names and dates for accuracy and completeness.
tests/countries/test_andorra.py Replaced broad year-specific tests with detailed, granular tests covering individual holidays, subdivisions, and categories over 1934–2050; added tests for government-only holidays and localization in Catalan, English, and Ukrainian; restructured setup to instantiate subdivision and government holiday objects for precise assertions.

Suggested labels

l10n, test

Suggested reviewers

  • PPsyrius
✨ Finishing Touches
  • 📝 Generate Docstrings

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.
    • Generate unit testing code for this file.
    • 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. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

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.

Documentation and Community

  • 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.

Copy link

codecov bot commented May 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5cbd555) to head (7ae8e31).
Report is 12 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #2530   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          215       215           
  Lines        13663     13676   +13     
  Branches      1930      1933    +3     
=========================================
+ Hits         13663     13676   +13     

☔ 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 5cbd555 and cc515a9.

📒 Files selected for processing (14)
  • README.md (1 hunks)
  • holidays/countries/andorra.py (4 hunks)
  • holidays/groups/christian.py (3 hunks)
  • holidays/locale/ca/LC_MESSAGES/AD.po (1 hunks)
  • holidays/locale/en_US/LC_MESSAGES/AD.po (1 hunks)
  • holidays/locale/uk/LC_MESSAGES/AD.po (1 hunks)
  • snapshots/countries/AD_02.json (1 hunks)
  • snapshots/countries/AD_03.json (1 hunks)
  • snapshots/countries/AD_04.json (1 hunks)
  • snapshots/countries/AD_05.json (1 hunks)
  • snapshots/countries/AD_07.json (1 hunks)
  • snapshots/countries/AD_08.json (1 hunks)
  • snapshots/countries/AD_COMMON.json (1 hunks)
  • tests/countries/test_andorra.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/countries/test_andorra.py (2)
holidays/countries/andorra.py (3)
  • Andorra (19-200)
  • AD (203-204)
  • AND (207-208)
tests/common.py (5)
  • assertAliases (121-130)
  • assertHolidayName (195-199)
  • assertNoHolidayName (273-275)
  • assertHolidays (228-230)
  • assertLocalizedHolidays (327-338)
🔇 Additional comments (37)
snapshots/countries/AD_COMMON.json (1)

4-1370: Bulk update to Andorra common holiday snapshot.
The carnival dates have been uniformly shifted one day earlier and holiday names like “Immaculate Conception” and “Our Lady of Meritxell” have been updated to match the new logic in the Andorra module. These changes align with the localization and date calculations in the country code.

holidays/locale/en_US/LC_MESSAGES/AD.po (1)

1-125: Added English (en_US) translations for Andorra holidays.
The new AD.po provides full msgid/msgstr pairs for all Catalan source entries and integrates cleanly with the tr calls in the Andorra country module under the en_US locale.

README.md (1)

181-181: Update supported languages for Andorra in README.
Andorra now correctly lists ca, en_US, uk as supported languages, matching the new localization files in the repository.

holidays/groups/christian.py (3)

198-206: Docstring correction for Carnival Tuesday.
The docstring now accurately states “Add Carnival Tuesday (47 days before Easter Sunday)”, which matches the _timedelta(..., -47) implementation.


372-377: Fix whitespace in Day Of Rejoicing docstring.
Minor formatting tweak improves readability around the Pascha reference; the holiday definition remains correct.


422-429: Standardize Saints Peter and Paul docstring.
Updated phrasing “Add Feast of Saints Peter and Paul (June 29th)” aligns with other feast entries and matches the _add_holiday_jun_29 call.

holidays/locale/uk/LC_MESSAGES/AD.po (1)

1-125: Added Ukrainian (uk) translations for Andorra holidays.
The new AD.po covers all holiday entries from the Catalan source and supplies accurate Ukrainian msgstr values, supporting the uk locale as expected.

snapshots/countries/AD_07.json (4)

4-4: Consistent Carnival date shift
The Carnival holiday has been moved one day earlier, aligning with the updated rule.


9-11: Standardize 'Andorra la Vella Festival' entries
The three-day festival entries are now correctly named and grouped for the parish.


13-13: Rename National Day to Our Lady of Meritxell
The holiday name now reflects the official designation.


15-15: Simplify 'Immaculate Conception' naming
Removed the 'Day' suffix for consistency across all locales.

snapshots/countries/AD_03.json (6)

4-4: Shift Carnival date for 1950
Carnival has been correctly moved one day earlier in this snapshot.


9-10: Consolidate Assumption Day and remove festival reference
The Encamp festival tag has been removed, leaving only 'Assumption Day'.


12-12: Rename 'Immaculate Conception'
Dropped the 'Day' suffix to match the updated naming convention.


18-18: Shift Carnival date for 1951
The one-day-earlier adjustment for Carnival is consistently applied.


23-24: Update Assumption Day and National Day naming for 1951
Festival removed and 'Our Lady of Meritxell' naming applied.


26-26: Standardize 'Immaculate Conception' naming for 1951
Consistent naming applied across snapshots.

snapshots/countries/AD_04.json (7)

4-5: Add Saint Anthony's Day for La Massana
The new parish-specific holiday on January 17 is correctly included.


5-5: Shift Carnival date for 1950
Carnival is moved one day earlier, consistent with the code logic.


10-11: Consolidate Assumption Day and rename National Day
Removed the extra festival entry and applied 'Our Lady of Meritxell' naming.


13-13: Rename 'Immaculate Conception'
Removed the 'Day' suffix for consistency with other entries.


18-19: Apply 1951 parish-specific and Carnival updates
Saint Anthony's Day and the Carnival adjustment are correctly repeated for 1951.


24-25: Apply 1951 rename updates
Assumption Day and Our Lady of Meritxell naming are consistently applied.


27-27: Standardize 'Immaculate Conception' for 1951
Consistent naming carried through across all years.

snapshots/countries/AD_08.json (4)

4-4: Carnival date shift approved.
The Carnival holiday dates have been moved one day earlier (e.g. from Feb 21 to Feb 20 in 1950) to match the official Andorra calendar.


10-10: Holiday rename approved.
Renamed “National Day” to “Our Lady of Meritxell” for September 8 in 1950, aligning with local tradition.


12-12: Shortened holiday name approved.
Changed “Immaculate Conception Day” to “Immaculate Conception” for consistency and brevity.


385-390: Subdivision-specific festival additions look good.
New entries for “Saint Michael of Engolasters’ Day”, “Parish foundation day” and the two-day “Escaldes-Engordany Festival” have been correctly introduced starting in 1979.

snapshots/countries/AD_05.json (3)

4-4: Carnival date shift approved.
The Carnival date in this variant has been aligned one day earlier, consistent with the other AD snapshots.


9-11: Saint and festival entries approved.
Added “Saint Peter’s Day” on June 29, retained “Assumption Day” on August 15, and renamed September 8 to “Our Lady of Meritxell” to improve specificity.


13-13: Shortened holiday name approved.
Standardized “Immaculate Conception” across all years for a cleaner label.

snapshots/countries/AD_02.json (3)

4-4: Carnival date shift approved.
Carnival moved one day earlier for 1950, matching the updated convention.


10-11: Saint Roch and National Day entries approved.
Introduced “Saint Roch’s Day” on August 16 and renamed September 8 to “Our Lady of Meritxell” to reflect official observances.


13-13: Shortened holiday name approved.
Renamed “Immaculate Conception Day” to “Immaculate Conception” for consistency.

tests/countries/test_andorra.py (2)

48-48: Double-check assertHolidayName(..., range(...)) signature

Most assertHolidayName calls provide explicit date strings, but here a bare range is passed.
If tests.common._assertHolidayName treats integers as years this is fine; otherwise the assertion will silently fail or mis-report.
Please verify the helper’s contract or switch to explicit date strings for consistency.


356-387:

✅ Verification successful

Verify that English l10n strings match the new .po catalog

A few English names (e.g., “Virgin Mary of Canòlich”, “Parish foundation day”) are uncommon; the .po file might choose slightly different wording or punctuation.
A quick grep can confirm the exact translation keys:

Adjust the expected test strings if the catalog differs.


🏁 Script executed:

#!/bin/bash
# List untranslated or mismatching English strings for Andorra (AD)
fd -e po 'AD.po' | xargs -I{} msggrep --msgstr-empty --output=/dev/null {} || true
grep -E "Canòlich|foundation day" $(fd -e po 'AD.po')

Length of output: 1021


English localization strings are in sync with AD.po
The test expectations for “Virgin Mary of Canòlich” and “Parish foundation day” exactly match the entries in holidays/locale/en_US/LC_MESSAGES/AD.po. No adjustments to the tests are needed.

holidays/countries/andorra.py (1)

184-185: Boundary year 1978: confirm exclusion logic

The comment says the parish was created on 14 Jun 1978 yet the guard if self._year <= 1978: excludes 1978 itself.
If the foundation celebrations started the following year this is correct; if they began during the year of creation (on or after 14 Jun 1978) you’ll miss that inaugural observance.

Just flagging in case historical sources differ.

Copy link

sonarqubecloud bot commented May 6, 2025

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (5)
snapshots/countries/AD_03.json (4)

3-7: Ensure consistency in government half-day holiday annotations
The half-day entries such as "Epiphany Eve (from 1pm)" and "New Year's Eve (from 1pm)" should match the conventions and parsing logic validated in the common snapshot.

Also applies to: 15-18


5-5: Verify Carnival date adjustments
Confirm that Carnival dates in this subdivision snapshot are shifted one day earlier in line with the main logic.


12-12: Confirm rename to Our Lady of Meritxell
Ensure "Our Lady of Meritxell" replaces any old “National Day” entries here and in translation files.


14-14: Validate shortened Immaculate Conception naming
Check that "Immaculate Conception" is used consistently and matches the common snapshot.

holidays/countries/andorra.py (1)

156-158: Empty block still lacks the promised explanation

_populate_subdiv_03_public_holidays() contains a bare pass without the
contextual comment requested in previous reviews.
A one-liner like # no parish-wide holidays (see BOA YYYY/XX) prevents future
contributors from wondering whether this is a TODO.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between cc515a9 and 7ae8e31.

📒 Files selected for processing (11)
  • README.md (1 hunks)
  • holidays/countries/andorra.py (4 hunks)
  • holidays/locale/ca/LC_MESSAGES/AD.po (1 hunks)
  • holidays/locale/en_US/LC_MESSAGES/AD.po (1 hunks)
  • holidays/locale/uk/LC_MESSAGES/AD.po (1 hunks)
  • snapshots/countries/AD_02.json (1 hunks)
  • snapshots/countries/AD_03.json (1 hunks)
  • snapshots/countries/AD_04.json (1 hunks)
  • snapshots/countries/AD_05.json (1 hunks)
  • snapshots/countries/AD_COMMON.json (1 hunks)
  • tests/countries/test_andorra.py (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
holidays/locale/ca/LC_MESSAGES/AD.po (8)
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: holidays/locale/ca/LC_MESSAGES/AD.po:31-40
Timestamp: 2025-05-06T15:25:44.299Z
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.
Learnt from: PPsyrius
PR: vacanza/holidays#2438
File: holidays/locale/ar_IQ/LC_MESSAGES/IQ.po:35-81
Timestamp: 2025-04-17T17:08:48.082Z
Learning: In holiday PO files, when the file represents the default language of an entity (e.g., ar_IQ for Iraq), no translations in `msgstr` are required as the `msgid` values are already in the target language.
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.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2465
File: holidays/locale/nl/LC_MESSAGES/SR.po:31-32
Timestamp: 2025-04-13T19:11:32.337Z
Learning: In the holidays library, when a language is set as the default language for a country (like Dutch/nl for Suriname), the `msgid` strings in the PO file are already in that language, and the corresponding `msgstr` fields are intentionally left empty. This is by design, as the system will fall back to using the `msgid` value when no translation is provided.
Learnt from: PPsyrius
PR: vacanza/holidays#2388
File: holidays/locale/fr/LC_MESSAGES/CI.po:28-101
Timestamp: 2025-03-30T13:33:31.598Z
Learning: In the holidays library, for localization files of the default language (like French for Ivory Coast in fr/LC_MESSAGES/CI.po), the best practice is to leave the message strings (msgstr) empty to avoid possible typos, since the message IDs (msgid) are already in the target language.
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.
Learnt from: KJhellico
PR: vacanza/holidays#2398
File: holidays/locale/fr/LC_MESSAGES/GN.po:31-83
Timestamp: 2025-03-31T20:25:12.808Z
Learning: In the holidays library, French (fr) is the default language for Guinea. This means that message IDs (msgid) in the PO files are already in French, and message strings (msgstr) in the French locale files can remain empty, as no translation is needed.
holidays/countries/andorra.py (1)
Learnt from: KJhellico
PR: vacanza/holidays#2530
File: holidays/countries/andorra.py:13-14
Timestamp: 2025-05-06T15:29:31.883Z
Learning: In this codebase, `tr` imported as `from gettext import gettext as tr` is used as a message marker for extracting translatable strings when generating .po files, not just for runtime translation.
tests/countries/test_andorra.py (2)
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.
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.
🔇 Additional comments (29)
README.md (1)

181-182: Andorra localization entries updated
The supported languages (ca, en_US, uk) and the addition of the GOVERNMENT category for Andorra are correctly reflected in the table.

holidays/locale/uk/LC_MESSAGES/AD.po (3)

1-29: PO header metadata verification
The file header includes correct Project-Id-Version, Language: uk, and generation timestamps, matching project conventions.


30-125: Ukrainian translations completeness
All Catalan msgid entries have accurate Ukrainian msgstr translations, covering public, religious, and subdivision-specific holidays.


126-130: Placeholder translation (c-format) correct
The Catalan placeholder %s (a partir de les 13h) is correctly translated to %s (з 13:00), preserving the format specifier.

holidays/locale/en_US/LC_MESSAGES/AD.po (3)

1-29: PO header metadata is correct
The header reflects Language: en_US, version, timestamps, and generator info, in line with other locale files.


30-89: English translations cover all holidays
Each Catalan msgid has a precise English msgstr, ensuring full coverage of Andorran holiday names.


126-130: Preserved c-format placeholder
The %s (a partir de les 13h) msgid is accurately translated as %s (from 1pm), with format specifier intact.

holidays/locale/ca/LC_MESSAGES/AD.po (3)

1-29: PO header metadata verification
The header is configured with Language: ca, correct project version, creation and revision dates, following defaults for Catalan.


30-77: Default locale entries intentionally empty
As Catalan is the default language, all msgstr fields are empty by design, enabling fallback to the msgid values.


126-130: c-format entry follows default locale pattern
The Catalan %s (a partir de les 13h) placeholder has an empty msgstr, consistent with default-locale conventions.

snapshots/countries/AD_04.json (4)

3-7: Add Epiphany Eve & local festivals
Entries for Jan 5 ("Epiphany Eve (from 1pm)"), Jan 17 ("Saint Anthony's Day"), and Carnival are correctly added.


7-9: Maundy Thursday half-day
"Maundy Thursday (from 1pm)" on the Thursday before Good Friday is properly marked as a half-day government holiday.


12-16: Consolidate Assumption, Meritxell & Immaculate
"Assumption Day" (Aug 15), the renamed "Our Lady of Meritxell" (Sep 8), and "Immaculate Conception" (Dec 8) entries match the updated naming requirements.


16-20: Year-end half-day additions
"Christmas Eve (from 1pm)" on Dec 24 and "New Year's Eve (from 1pm)" on Dec 31 correctly reflect introduced half-day observances.

snapshots/countries/AD_COMMON.json (4)

12-12: Confirm rename to Our Lady of Meritxell
The holiday formerly known as “National Day” is now "Our Lady of Meritxell". Check that all translation files (holidays/locale/*/LC_MESSAGES/AD.po) and code references have been updated accordingly.


14-14: Validate shortened Immaculate Conception naming
Changed from “Immaculate Conception Day” to "Immaculate Conception". Ensure the code and localization keys reflect this new name and no legacy references remain.


3-7:

❓ Verification inconclusive

Check government half-day holiday entries consistency
The newly added entries "Epiphany Eve (from 1pm)", "Maundy Thursday (from 1pm)", "Christmas Eve (from 1pm)", and "New Year's Eve (from 1pm)" must follow a uniform naming pattern and align with the parsing logic in andorra.py. Confirm that the suffix "(from 1pm)" is correctly recognized across locales and in tests.
Run:

Also applies to: 15-18


🏁 Script executed:

rg -n '"(from 1pm)" snapshots/countries/AD_COMMON.json | head -n 5

Length of output: 116


Let’s correct the quoting and verify both the JSON snapshots and the parsing logic:


🏁 Script executed:

#!/usr/bin/env bash
set -euxo pipefail

# 1. Search for the “(from 1pm)” suffix in AD_COMMON.json
rg -n "\\(from 1pm\\)" snapshots/countries/AD_COMMON.json

# 2. Ensure it’s consistent across all country snapshot files
rg -n "\\(from 1pm\\)" snapshots/countries/

# 3. Locate and check how “from 1pm” is handled in the Andorra parser
ANDORRA_PY=$(fd --glob andorra.py | head -n1)
rg -n "from 1pm" "$ANDORRA_PY"

Length of output: 119455


Government half-day suffix uniformity verified
We’ve confirmed the “(from 1pm)” suffix appears on Epiphany Eve, Maundy Thursday, Christmas Eve, and New Year’s Eve across all AD snapshots (AD_COMMON.json, AD_02.json, AD_06.json). The parser (holidays/countries/andorra.py) doesn’t hard-code the literal suffix—it uses metadata to generate half-day labels.

• Snapshots checked:

  • snapshots/countries/AD_COMMON.json
  • snapshots/countries/AD_02.json
  • snapshots/countries/AD_06.json

• Parser file: holidays/countries/andorra.py (no “from 1pm” literal)

Please verify that the metadata-based half-day support in andorra.py produces the “(from 1pm)” suffix correctly in all locales and that existing tests include these half-day entries.


5-5:

✅ Verification successful

Verify Carnival date adjustments
Carnival has been shifted one day earlier ("1950-02-20" vs previous Tuesday). Ensure the holiday-population logic in the module applies this shift consistently for all years.


🏁 Script executed:

rg -n '"Carnival"' snapshots/countries/AD_COMMON.json | head -n 1

Length of output: 99


Let’s verify the weekday for every “Carnival” entry in snapshots/countries/AD_COMMON.json:


🏁 Script executed:

#!/usr/bin/env bash
python3 - << 'EOF'
import json, datetime, collections
data = json.load(open('snapshots/countries/AD_COMMON.json'))
carnival_dates = [d for d, name in data.items() if name == 'Carnival']
counts = collections.Counter(
    datetime.datetime.strptime(d, '%Y-%m-%d').weekday() for d in carnival_dates
)
# weekday: 0=Mon, 1=Tue, … 6=Sun
print("weekday_counts:", counts)
EOF

Length of output: 45


Confirmed consistent shift for Carnival
All “Carnival” entries in snapshots/countries/AD_COMMON.json fall on Monday (weekday_counts: {0:101}), so the one-day earlier adjustment is applied uniformly across all years. No further changes needed.

snapshots/countries/AD_02.json (5)

2-19: Consistent addition of half-day holidays
The new entries "Epiphany Eve (from 1pm)", "Maundy Thursday (from 1pm)", "Christmas Eve (from 1pm)", and "New Year's Eve (from 1pm)" are correctly inserted for the year 1950. This pattern is repeated systematically for each subsequent year, matching the updated government holiday definitions.


12-13: "Saint Roch's Day" inclusion
The annual addition of "Saint Roch's Day" on August 16 for 1950 is present and correctly formatted. Ensure this holiday appears for all years in the same position.


13-14: Renamed National Day
The renaming from "National Day" to "Our Lady of Meritxell" on September 8 is accurately applied, aligning with the refined holiday naming.


15-16: Standardized Immaculate Conception
The change from "Immaculate Conception Day" to "Immaculate Conception" on December 8 reflects the l10n conventions and is consistently applied.


2-1876:

❓ Verification inconclusive

Snapshot completeness & order
The JSON remains in strict chronological order for all years 1950–2050. To guard against drift, please regenerate the Andorra snapshot via the build tool and diff it against this file.

Run:


🏁 Script executed:

holidays generate --country AD --snapshot-version 02 > /tmp/AD_02_new.json
diff -u snapshots/countries/AD_02.json /tmp/AD_02_new.json

Length of output: 65751


Snapshot regeneration unavailable in this environment
I attempted to rerun the build tool (holidays generate) but the CLI isn’t installed here, so the new snapshot came up empty. Please verify locally that the Andorra snapshot is still complete and chronologically ordered:

holidays generate --country AD --snapshot-version 02 > /tmp/AD_02_new.json
diff -u snapshots/countries/AD_02.json /tmp/AD_02_new.json

Ensure there are no unexpected changes, and commit an updated snapshot if you spot any drift.

snapshots/countries/AD_05.json (4)

3-19: Half-day observances added
Entries for "Epiphany Eve (from 1pm)", "Maundy Thursday (from 1pm)", "Christmas Eve (from 1pm)", and "New Year's Eve (from 1pm)" appear correctly for 1950 and follow the same pattern in later years.


11-13: "Saint Peter's Day" & "Assumption Day" split
The snapshot now lists "Saint Peter's Day" (June 29) separately from "Assumption Day" (August 15), which reflects the refined subdivision-specific holiday logic.


12-14: Renaming to Our Lady of Meritxell
The substitution of the old "National Day" with "Our Lady of Meritxell" on September 8 is correctly implemented in the 1950 block and repeats for all years.


1-1376:

❓ Verification inconclusive

Verify snapshot accuracy
The JSON ordering and content look correct. Please regenerate the AD-05 snapshot and diff to ensure full alignment with code output.

Run:


🏁 Script executed:

holidays generate --country AD --snapshot-version 05 > /tmp/AD_05_new.json
diff -u snapshots/countries/AD_05.json /tmp/AD_05_new.json

Length of output: 73898


Unable to auto-generate snapshot in CI
The holidays generate command wasn’t found in the sandbox, so we couldn’t diff the regenerated AD-05 snapshot. Please verify locally:

  • Install or expose the CLI (e.g. pip install holidays-cli).
  • Regenerate and compare:
    holidays generate --country AD --snapshot-version 05 > /tmp/AD_05_new.json
    diff -u snapshots/countries/AD_05.json /tmp/AD_05_new.json

Ensure there are no unexpected diffs before merging.

holidays/countries/andorra.py (1)

188-191: Double-check the cut-off year for Escaldes-Engordany

The parish was created on 14 Jun 1978, yet all holiday logic is skipped when
self._year <= 1978.
That means 1978 itself will have zero parish holidays even after its
creation date. Was that intentional? If not, switch to < 1978 or add a
special-case for the second half of 1978.

tests/countries/test_andorra.py (1)

52-54: Confirm that assertHolidayName() accepts raw range objects

Several calls pass a bare range(...) (not a generator of date strings).
If assertHolidayName() expects date strings, this will silently skip
verification. Please run the helper with an explicit list or update the helper
to recognise year ranges.

Example fix if needed:

- self.assertHolidayName(name, range(1934, 2050))
+ self.assertHolidayName(name, (f"{y}-mm-dd" for y in self.years))  # adjust fmt

Replace mm-dd with the appropriate fixed month/day for the holiday.

Also applies to: 70-72, 83-85, 101-103, 154-156, 165-168, 185-186, 210-213, 241-243, 251-254, 263-280, 305-307, 330-331, 337-344

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

@arkid15r arkid15r enabled auto-merge May 14, 2025 23:11
Copy link
Collaborator

@PPsyrius PPsyrius left a comment

Choose a reason for hiding this comment

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

LGTM 🇦🇩

@arkid15r arkid15r added this pull request to the merge queue May 15, 2025
Merged via the queue into vacanza:dev with commit 7027010 May 15, 2025
33 checks passed
@KJhellico KJhellico deleted the upd-andorra branch May 15, 2025 11:11
@KJhellico KJhellico mentioned this pull request May 19, 2025
Wasif-Shahzad pushed a commit to Wasif-Shahzad/holidays that referenced this pull request May 21, 2025
ShalomDee pushed a commit to ShalomDee/holidays that referenced this pull request Jun 2, 2025
@coderabbitai coderabbitai bot mentioned this pull request Jul 9, 2025
9 tasks
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