+
Skip to content

Conversation

PPsyrius
Copy link
Collaborator

Proposed change

  • Reduce l10n string duplication in Japan's holiday code.
  • Fix Japan's en_US l10n from "Autumnal Equinox" to "Autumnal Equinox Day"

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

Copy link
Contributor

coderabbitai bot commented Sep 17, 2025

Summary by CodeRabbit

  • New Features

    • None
  • Bug Fixes

    • Corrected Emperor’s Birthday dates across eras and excluded 2019 as a special case.
    • Standardized holiday naming to “Autumnal Equinox Day.”
  • Refactor

    • Simplified logic for Showa Day and Greenery Day, consolidating era-based rules.
  • Localization

    • Updated translations and metadata for English, Japanese, and Thai.
    • Reordered Greenery Day entry for consistency without changing translations.
  • Tests

    • Updated expected holiday name to “Autumnal Equinox Day.”

Walkthrough

Unifies Emperor’s Birthday, Showa Day, and Greenery Day logic in Japan holidays. Updates English, Japanese, and Thai PO metadata and reorders Greenery Day entries. Renames “Autumnal Equinox” to “Autumnal Equinox Day” across en_US PO, snapshots, and a test expectation.

Changes

Cohort / File(s) Summary
JP holiday rules
holidays/countries/japan.py
Consolidates Emperor’s Birthday into single era-aware block (skips 2019); simplifies Showa Day handling; unifies Greenery Day date/name logic for pre/post-2007.
Localizations (en, ja, th)
holidays/locale/en_US/LC_MESSAGES/JP.po, holidays/locale/ja/LC_MESSAGES/JP.po, holidays/locale/th/LC_MESSAGES/JP.po
Update PO headers; reorder Greenery Day block position; in en_US, rename “Autumnal Equinox” → “Autumnal Equinox Day”.
Snapshot data
snapshots/countries/JP_COMMON.json
Rename holiday label “Autumnal Equinox” → “Autumnal Equinox Day” in data entries.
Tests
tests/countries/test_japan.py
Adjust expected en_US name for 2022-09-23 to “Autumnal Equinox Day”.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

l10n, snapshot, test

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 PR title clearly and concisely describes the main work in the changeset: a refactor to reduce Japan localization string duplication and a correction to the en_US label for the Autumnal Equinox, and it matches the modified files and tests in the diff.
Description Check ✅ Passed The PR description succinctly states the intent and scope—l10n deduplication, the en_US string fix, relation to the earlier PR, and that checks were run—making it directly related to the changeset and not off-topic.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent 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 d8ecdbe and b950a33.

📒 Files selected for processing (6)
  • holidays/countries/japan.py (1 hunks)
  • holidays/locale/en_US/LC_MESSAGES/JP.po (3 hunks)
  • holidays/locale/ja/LC_MESSAGES/JP.po (2 hunks)
  • holidays/locale/th/LC_MESSAGES/JP.po (2 hunks)
  • snapshots/countries/JP_COMMON.json (101 hunks)
  • tests/countries/test_japan.py (1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📚 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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.po
📚 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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.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/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
  • holidays/locale/th/LC_MESSAGES/JP.po
📚 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/locale/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
📚 Learning: 2025-05-10T04:34:02.406Z
Learnt from: PPsyrius
PR: vacanza/holidays#2537
File: holidays/locale/sv_FI/LC_MESSAGES/AX.po:17-27
Timestamp: 2025-05-10T04:34:02.406Z
Learning: The `Plural-Forms` header isn't used in .po file generation for the holidays project and doesn't need to be manually added to localization files.

Applied to files:

  • holidays/locale/ja/LC_MESSAGES/JP.po
  • holidays/locale/en_US/LC_MESSAGES/JP.po
📚 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/locale/th/LC_MESSAGES/JP.po
📚 Learning: 2025-08-28T02:34:11.657Z
Learnt from: PPsyrius
PR: vacanza/holidays#2860
File: holidays/groups/eastern.py:36-36
Timestamp: 2025-08-28T02:34:11.657Z
Learning: The default estimated_label change from "%s (estimated)" to "%s" in holidays/groups/eastern.py is intentional to remove English-only fallback text from localized holiday names. Countries that need estimated labels should define them properly with localization support rather than relying on the English fallback.

Applied to files:

  • holidays/locale/th/LC_MESSAGES/JP.po
📚 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:

  • holidays/countries/japan.py
🧬 Code graph analysis (1)
holidays/countries/japan.py (1)
holidays/holiday_base.py (1)
  • _add_holiday (797-809)
⏰ 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). (2)
  • GitHub Check: Test Python 3.14 on windows-latest
  • GitHub Check: Test Python 3.13 on windows-latest
🔇 Additional comments (12)
holidays/locale/en_US/LC_MESSAGES/JP.po (3)

17-27: Headers updated correctly; no regeneration needed.

Metadata bumps (Project-Id-Version, PO-Revision-Date, Last-Translator, X-Generator) are valid per project conventions.


66-69: Greenery Day block relocation is fine.

Reordering aligns with code refactor; translations remain correct.


88-89: Approve: rename to "Autumnal Equinox Day" is correct.
No remaining occurrences of "Autumnal Equinox" (without "Day") in snapshots/countries/JP_COMMON.json, tests/countries/test_japan.py, or holidays/locale/en_US/LC_MESSAGES/JP.po.

holidays/locale/th/LC_MESSAGES/JP.po (2)

17-27: Headers updated correctly; no regeneration needed.

Consistent with l10n metadata policy.


66-69: Greenery Day block relocation is fine.

Ordering mirrors JP logic; Thai translations unchanged.

holidays/countries/japan.py (3)

119-122: Showa Day gating (>= 2007) avoids duplication with pre‑1989 Emperor’s Birthday.

Matches legal history and tests.


126-134: Greenery Day consolidation is correct.

1989–2006 on Apr 29; 2007+ on May 4; single localized name.


102-115: Emperor’s Birthday unified logic is correct — 2019 intentionally skipped.
Snapshot verification: JP_COMMON.json has no "Emperor's Birthday" in 2019; it records "Emperor's Enthronement Day" on 2019-05-01 and 2019-10-22.

tests/countries/test_japan.py (1)

779-779: Test expectation updated to “Autumnal Equinox Day” — good.

Aligned with en_US PO and snapshots.

snapshots/countries/JP_COMMON.json (1)

10-10: Approve — bulk rename to "Autumnal Equinox Day" is consistent.
Data-only change; no structural diffs — verified 101 occurrences in snapshots/countries/JP_COMMON.json and no instances of 'Autumnal Equinox' without ' Day' remain.

holidays/locale/ja/LC_MESSAGES/JP.po (2)

66-69: Greenery Day moved to correct chronological position.

Now ordered as 昭和の日 (4/29) → 憲法記念日 (5/3) → みどりの日 (5/4) → こどもの日 (5/5). Looks right.


15-29: Header metadata OK — verify Project-Id-Version

  • All msgstr entries are empty in holidays/locale/ja/LC_MESSAGES/JP.po — OK.
  • Header contains Project-Id-Version: Holidays 0.82; JP.pot not found in the repository, so cannot cross-check. Confirm 0.82 matches the current POT version.

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/japan.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

tests/countries/test_japan.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.

Copy link

Copy link

codecov bot commented Sep 17, 2025

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #2937   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          296       296           
  Lines        17596     17597    +1     
  Branches      2270      2270           
=========================================
+ Hits         17596     17597    +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.

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!

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.

👍

@arkid15r arkid15r added this pull request to the merge queue Sep 17, 2025
Merged via the queue into vacanza:dev with commit 9d75edc Sep 17, 2025
36 checks passed
@PPsyrius PPsyrius deleted the jp_refactor branch September 18, 2025 02:28
@arkid15r arkid15r mentioned this pull request Oct 6, 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

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载