python-holidays icon indicating copy to clipboard operation
python-holidays copied to clipboard
verified publisher icon vacanza

Update Aruba, Curaçao, Netherlands holidays: general code refactor

Open PPsyrius opened this issue 6 months ago • 3 comments

Proposed change

  • Update Aruba, Curaçao, and the Netherlands observance code for King's/Queen's Day based on #2475 approach.
  • Update Aruba and Curaçao codebase and test case to modern library-wide standard (start year, reference sources, ...)

Type of change

  • [ ] New country/market holidays support (thank you!)
  • [ ] Supported country/market holidays update (calendar discrepancy fix, localization)
  • [x] 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

  • [x] I've followed the contributing guidelines
  • [x] I've successfully run make check, all checks and tests are green

PPsyrius avatar Jun 25 '25 10:06 PPsyrius

Summary by CodeRabbit

  • New Features

    • Improved handling of observed holidays for Aruba, Curacao, and the Netherlands, including automatic shifting of holidays that fall on Sundays.
    • Added new holidays for Curacao: Whit Sunday and Kingdom Day.
    • Updated holiday date ranges and observance rules to reflect historical accuracy.

  • Localization

    • Enhanced and standardized translations for Aruba and Curacao holidays across English, Dutch, Papiamento, and Ukrainian.
    • Updated translations for new and existing holidays, including improved naming consistency.

  • Bug Fixes

    • Corrected holiday dates and observance logic for Labor Day and King's/Queen's Day.
    • Fixed and clarified year ranges for various holidays.

  • Tests

    • Expanded and refined test coverage for Aruba and Curacao holidays, including new tests for specific holidays and improved year range checks.

  • Chores

    • Updated holiday data snapshots to reflect changes in holiday dates and additions.

Summary by CodeRabbit

  • New Features

    • Improved handling of observed holidays for Aruba, Curacao, and the Netherlands, ensuring holidays falling on Sundays are automatically moved according to local rules.
    • Added new holidays for Curacao: Whit Sunday and Kingdom Day, with corresponding localizations.
    • Updated translations for Aruba and Curacao holidays in English, Dutch, Papiamento, and Ukrainian, including refined names and added holidays.

  • Bug Fixes

    • Corrected holiday start years for Aruba and Curacao to reflect historical accuracy.
    • Adjusted holiday dates and observance rules, especially for Labor Day and King's/Queen's Day.

  • Tests

    • Expanded and refined test coverage for Aruba and Curacao holidays, including more precise year ranges and new holiday assertions.

  • Documentation

    • Standardized holiday descriptions and improved reference links in comments and translations.

Walkthrough

The changes refactor Aruba, Curacao, and Netherlands holiday classes to use ObservedHolidayBase for standardized observed holiday handling, update start years, and revise holiday logic for Labor Day and King's/Queen's Day. Localization files were updated for consistency and completeness, and test suites for Aruba and Curacao were expanded and refined to cover new and adjusted holiday rules.

Changes

Files/Groups Change Summary
holidays/countries/aruba.py, holidays/countries/curacao.py, holidays/countries/netherlands.py Refactored classes to inherit from ObservedHolidayBase with default observed_rule set. Updated start years (Aruba: 1947→1955, Curacao: 1954→1955). Holiday logic for Labor Day and King's/Queen's Day refactored to use _move_holiday and helper methods replacing manual date adjustments. Comments and references standardized; unused imports removed.
holidays/locale/en_US/LC_MESSAGES/AW.po, holidays/locale/nl/LC_MESSAGES/AW.po, holidays/locale/pap_AW/LC_MESSAGES/AW.po, holidays/locale/uk/LC_MESSAGES/AW.po Aruba holiday localizations updated: metadata versions bumped, holiday comment lines standardized with trailing periods, "Labor Day" entry added or repositioned, Dutch translation for "Christmas Day" refined. No changes to exported code entities.
holidays/locale/en_US/LC_MESSAGES/CW.po, holidays/locale/nl/LC_MESSAGES/CW.po, holidays/locale/pap_CW/LC_MESSAGES/CW.po, holidays/locale/uk/LC_MESSAGES/CW.po Curacao holiday localizations updated: metadata refreshed, comments standardized, new entries for "Whit Sunday" and "Kingdom Day" added, "Labor Day" repositioned, Dutch translation for "Christmas Day" refined. No changes to exported code entities.
snapshots/countries/AW_COMMON.json Adjusted "Labor Day" date from May 1 to May 2 for most years (1955–2050); split from combined "Queen's Day" where previously merged. No new holidays added or removed; JSON structure unchanged.
snapshots/countries/CW_COMMON.json Removed all 1954 entries and "New Year's Eve" from years after 2009. Added "Whit Sunday" (1955–2009) and "Kingdom Day" (2008–2009). Adjusted holiday combinations for some years. No code changes.
tests/countries/test_aruba.py Expanded test coverage: added tests for New Year's Day, Good Friday, Easter Monday, Labor Day, Ascension Day, Christmas. Refined year ranges for holiday presence and absence. Clarified holiday assertions and updated Dutch localization test for Christmas name.
tests/countries/test_curacao.py Expanded test coverage: added tests for New Year's Day, Carnival Monday, Good Friday, Easter Sunday, Easter Monday, Ascension Day, Whit Sunday, Christmas, New Year's Eve. Separated half-day and public holiday tests. Clarified year ranges for holiday presence and absence. Updated Dutch localization test.

Suggested reviewers

  • KJhellico
📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between f302491613bd5ee637f26928bc4bdfae90b25536 and 594a5c45c379ff52f5a703aed70822dc9520bf51.

📒 Files selected for processing (1)
  • tests/countries/test_aruba.py (7 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/countries/test_aruba.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.
🧬 Code Graph Analysis (1)
tests/countries/test_aruba.py (2)
tests/common.py (6)
  • setUpClass (32-53)
  • assertAliases (121-130)
  • assertNoHolidays (292-294)
  • assertHolidayName (195-199)
  • assertNoHolidayName (273-275)
  • assertNoHoliday (244-246)
holidays/countries/aruba.py (3)
  • Aruba (24-151)
  • AW (154-155)
  • ABW (158-159)
🪛 Pylint (3.3.7)
tests/countries/test_aruba.py

[convention] 24-24: Missing function or method docstring

(C0116)

[convention] 27-27: Missing function or method docstring

(C0116)

[convention] 46-46: Missing function or method docstring

(C0116)

[convention] 49-49: Missing function or method docstring

(C0116)

[convention] 79-79: Missing function or method docstring

(C0116)

[convention] 84-84: Missing function or method docstring

(C0116)

[convention] 97-97: Missing function or method docstring

(C0116)

[convention] 139-139: Missing function or method docstring

(C0116)

[convention] 172-172: Missing function or method docstring

(C0116)

[convention] 192-192: Missing function or method docstring

(C0116)

[convention] 205-205: Missing function or method docstring

(C0116)

⏰ Context from checks skipped due to timeout of 300000ms (5)
  • GitHub Check: Test Python 3.10 on windows-latest
  • GitHub Check: Test Python 3.11 on windows-latest
  • GitHub Check: Test Python 3.12 on windows-latest
  • GitHub Check: Test Python 3.9 on windows-latest
  • GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (8)
tests/countries/test_aruba.py (8)

22-22: Good alignment with updated start year.

The year range change from 1947 to 1955 correctly aligns with the updated start_year in the Aruba holiday class.

28-28: Proper test for no holidays before start year.

Testing 1954 (one year before the start_year of 1955) is the correct approach to verify no holidays exist before the defined start year.

46-48: Well-structured individual holiday test.

The new test method follows the preferred pattern from the retrieved learnings, focusing on a specific holiday and testing it across the full year range.

68-77: Comprehensive carnival monday name transition testing.

The test properly handles the name change from "Dialuna despues di Carnaval Grandi" to "Dialuna prome cu diaranson di shinish" in 2023, with correct year range assertions for both names.

79-83: Proper testing of holiday start year.

The test correctly validates that National Anthem and Flag Day starts from 1976 and doesn't exist before that year.

84-96: Excellent individual holiday test coverage.

The new test methods for Good Friday, Easter Monday, Labor Day, and Ascension Day follow best practices by testing specific dates and full year ranges, providing comprehensive coverage for these holidays.

Also applies to: 97-108, 172-191, 192-204

125-126: Correct mutual exclusivity for Queen's and King's Day.

The updated year ranges properly reflect the transition from Queen's Day (1955-2013) to King's Day (2014+), ensuring no overlap between the two holidays.

Also applies to: 137-137, 169-170

256-256: Updated Christmas Day translation.

The change to "Eerste Kerstdag" aligns with the localization updates in the main codebase.

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings
🧪 Generate Unit Tests
  • [ ] Create PR with Unit Tests
  • [ ] Post Copyable Unit Tests in 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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 auto-generate unit tests to generate unit tests for 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.

coderabbitai[bot] avatar Jun 25 '25 10:06 coderabbitai[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (29372bd) to head (594a5c4). Report is 1 commits behind head on dev.

Additional details and impacted files 
@@            Coverage Diff            @@
##               dev     #2677   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          251       251           
  Lines        15286     15272   -14     
  Branches      2095      2092    -3     
=========================================
- Hits         15286     15272   -14

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Jun 25 '25 10:06 codecov[bot]

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

sonarqubecloud[bot] avatar Jun 26 '25 02:06 sonarqubecloud[bot]

Hi, it seems this version has a breaking change (at least for the Netherlands).

Working code on version 0.75:

nl_holidays = []
for year in range(int(datetime.datetime.now(datetime.UTC).year), int(datetime.datetime.now(datetime.UTC).year) + 2):
      for ptr in holidays.NL(year, language="nl").items():
          nl_holidays.append({"date": ptr[0].isoformat(), "name": ptr[1]})

Running on version 0.76: Rrror:

  File "/Users/jovana/.pyenv/versions/3.12.9/envs/smartrental-api/lib/python3.12/site-packages/holidays/countries/netherlands.py", line 39, in __init__
    super().__init__(*args, **kwargs)
TypeError: ObservedHolidayBase.__init__() got multiple values for argument 'observed_rule'

This will fix this error on version 0.75:

holidays.NL(years=year, language="nl")

jovana avatar Jul 09 '25 11:07 jovana

@jovana check out the docs here or here (the years param). Using it as holidays.NL(year, language="nl") doesn't seem right.

Please next time file a new issue instead of commenting on a closed PR.

Thanks for using holidays!

arkid15r avatar Jul 09 '25 15:07 arkid15r