Refactor Hungary holidays: reduce l10n string duplication #2936

PPsyrius · 2025-09-16T08:26:14Z

Proposed change

Reduce l10n string duplication in Hungary's holiday code.

This is part of the non-test case changes decoupling from #2881

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

I've read and followed the contributing guidelines.
I've run make check locally; all checks and tests passed.

coderabbitai · 2025-09-16T08:26:21Z

Summary by CodeRabbit

Bug Fixes
- Ensured Hungary’s Oct 23 holiday appears only from 1991 onward without duplicate entries.
- Unified the translated name for Hungary’s National Days (Mar 15 and Oct 23) for consistent display.
Refactor
- Streamlined Hungary National Day handling to reduce redundancy and improve consistency.

Walkthrough

Refactors Hungary holiday logic: centralizes the translated National Day name, adds Mar 15 via a helper, and conditionally adds Oct 23 within the same block for years ≥1991. Removes the separate later block that previously added Oct 23, avoiding duplication. No public API changes.

Changes

Cohort / File(s)	Summary
Hungary holidays logic `holidays/countries/hungary.py`	Centralized translated name for National Day; Mar 15 added via `_add_holiday_mar_15(name)`; Oct 23 addition moved into the same conditional for years ≥1991 using the shared name; removed the later duplicate Oct 23 block.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

l10n

Suggested reviewers

arkid15r
KJhellico

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "Refactor Hungary holidays: reduce l10n string duplication" concisely and accurately summarizes the primary change in this PR — a refactor to remove duplicated localization strings in the Hungary holidays module; it is focused, specific to the affected area, and readable for teammates scanning history. It does not include noisy file lists or vague terms and aligns with the raw summary and PR objectives.
Description Check	✅ Passed	The PR description clearly states the intent to reduce l10n string duplication in Hungary's holiday code, references the related PR, marks the change as a refactor, and notes local checks passed, so it is directly related to the changeset and meets the lenient acceptance criteria. It provides sufficient context for reviewers even if it is not highly detailed.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Ruff (0.12.2)

holidays/countries/hungary.py

�[1;31mruff failed�[0m
�[1mCause:�[0m Failed to load configuration /ruff.toml
�[1mCause:�[0m Failed to parse /ruff.toml
�[1mCause:�[0m TOML parse error at line 26, column 3
|
26 | "RSE100", # Use of assert detected
| ^^^^^^^^
Unknown rule selector: RSE100

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

sonarqubecloud · 2025-09-16T08:27:16Z

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

codecov · 2025-09-16T08:28:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7347cd0) to head (162909c).
⚠️ Report is 5 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff            @@
##               dev     #2936   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          296       296           
  Lines        17593     17594    +1     
  Branches      2270      2270           
=========================================
+ Hits         17593     17594    +1

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

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7347cd0 and 162909c.

📒 Files selected for processing (1)

holidays/countries/hungary.py (1 hunks)

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: PPsyrius
PR: vacanza/holidays#2881
File: tests/countries/test_albania.py:40-42
Timestamp: 2025-09-04T08:54:35.319Z
Learning: In the vacanza/holidays project test files, extract holiday name strings to local variables only when they are reused multiple times within the same test method (e.g., used in both assertHolidayName and assertNoHolidayName calls). When a holiday name is used only once, keep it inline rather than extracting it to a variable for the sake of consistency.

Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
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.

Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:0-0
Timestamp: 2025-05-10T04:32:15.760Z
Learning: In the holidays package, detailed historical context and additional information should be added as comments at the method level or above conditional blocks, while comments directly above tr() function calls should only contain the holiday name itself (e.g., "# Independence Day.").

Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
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.

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.

Learnt from: PPsyrius
PR: vacanza/holidays#2632
File: holidays/countries/solomon_islands.py:95-98
Timestamp: 2025-06-16T12:28:31.641Z
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.

📚 Learning: 2025-04-03T16:58:27.175Z

Learnt from: Wasif-Shahzad
PR: vacanza/holidays#2409
File: holidays/countries/qatar.py:27-46
Timestamp: 2025-04-03T16:58:27.175Z
Learning: In the holidays library, method names like `_add_holiday_2nd_tue_of_feb()` and `_add_holiday_1st_sun_of_mar()` use calendar constants internally through parent class implementations even when these constants don't appear directly in the file. Removing imports that seem unused based on a simple text search could break functionality.

Applied to files:

holidays/countries/hungary.py

📚 Learning: 2025-08-21T04:56:03.780Z

Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
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/hungary.py

📚 Learning: 2025-08-21T04:56:03.780Z

Learnt from: PPsyrius
PR: vacanza/holidays#2843
File: holidays/countries/burundi.py:63-101
Timestamp: 2025-08-21T04:56:03.780Z
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/hungary.py

📚 Learning: 2025-05-10T04:32:15.760Z

Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/countries/finland.py:0-0
Timestamp: 2025-05-10T04:32:15.760Z
Learning: In the holidays package, detailed historical context and additional information should be added as comments at the method level or above conditional blocks, while comments directly above tr() function calls should only contain the holiday name itself (e.g., "# Independence Day.").

Applied to files:

holidays/countries/hungary.py

⏰ 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). (1)

GitHub Check: Test Python 3.13 on windows-latest

🔇 Additional comments (1)

holidays/countries/hungary.py (1)

46-49: Good consolidation of l10n and guards.

Using a single translated name for both Mar 15 and Oct 23 removes duplication, and the year conditions preserve prior behavior (Mar 15 for ≤1950 or ≥1989; Oct 23 for ≥1991).

holidays/countries/hungary.py

KJhellico

LGTM!

Refactor Hungary holidays: reduce l10n string duplication

162909c

PPsyrius requested review from KJhellico and arkid15r as code owners September 16, 2025 08:26

coderabbitai bot reviewed Sep 16, 2025

View reviewed changes

holidays/countries/hungary.py Show resolved Hide resolved

holidays/countries/hungary.py Show resolved Hide resolved

KJhellico approved these changes Sep 16, 2025

View reviewed changes

arkid15r approved these changes Sep 16, 2025

View reviewed changes

arkid15r added this pull request to the merge queue Sep 16, 2025

Merged via the queue into vacanza:dev with commit d8ecdbe Sep 16, 2025
36 checks passed

PPsyrius deleted the hungary_refactor branch September 17, 2025 02:58

arkid15r mentioned this pull request Oct 6, 2025

v0.82 #2988

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Refactor Hungary holidays: reduce l10n string duplication #2936

Refactor Hungary holidays: reduce l10n string duplication #2936

Uh oh!

PPsyrius commented Sep 16, 2025

Uh oh!

coderabbitai bot commented Sep 16, 2025 •

edited

Loading

Uh oh!

sonarqubecloud bot commented Sep 16, 2025

Uh oh!

codecov bot commented Sep 16, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

KJhellico left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Refactor Hungary holidays: reduce l10n string duplication #2936

Refactor Hungary holidays: reduce l10n string duplication #2936

Uh oh!

Conversation

PPsyrius commented Sep 16, 2025

Proposed change

Type of change

Checklist

Uh oh!

coderabbitai bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

sonarqubecloud bot commented Sep 16, 2025

Quality Gate passed

Uh oh!

codecov bot commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KJhellico left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Sep 16, 2025 •

edited

Loading

codecov bot commented Sep 16, 2025 •

edited

Loading