feat: Add NYSE half-day holidays #2992
Conversation
Signed-off-by: Kartik Agarwal <kartikk1407@gmail.com>
Summary by CodeRabbit
WalkthroughAdds HALF_DAY constant import. Introduces NewYorkStockExchange._populate_half_day_holidays to register NYSE half-day holidays (pre–Independence Day, day after Thanksgiving, Christmas Eve when Christmas Day is a weekday). Invokes this method from _populate_public_holidays to include half-day entries without altering existing full-day holiday definitions. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
holidays/financial/ny_stock_exchange.py(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
holidays/financial/ny_stock_exchange.py (5)
holidays/countries/china.py (1)
_populate_half_day_holidays(190-203)holidays/countries/turkey.py (1)
_populate_half_day_holidays(109-120)holidays/countries/curacao.py (1)
_populate_half_day_holidays(159-166)holidays/countries/greece.py (1)
_populate_half_day_holidays(91-96)holidays/countries/iceland.py (1)
_populate_half_day_holidays(82-90)
🔇 Additional comments (2)
holidays/financial/ny_stock_exchange.py (2)
32-32: LGTM on the import.The
HALF_DAYconstant import is appropriate for the half-day holiday implementation.
144-162: PR objectives mismatch.The PR claims to fix issue #2876, which is about expanding label tests for financial entities. However, this PR adds actual half-day holiday functionality rather than test updates. The linked issue and the actual changes don't align.
| # XMAS DAY: Dec 25th - every year | ||
| self._move_holiday(self._add_christmas_day("Christmas Day")) | ||
|
|
||
| self._populate_half_day_holidays() |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the class structure.
The method call looks correct, but the method definition at line 144 has a critical indentation issue that will cause an AttributeError at runtime.
Run this script to confirm the indentation problem:
🏁 Script executed:
#!/bin/bash
# Verify that _populate_half_day_holidays is properly indented as a class method
# Check the indentation of the method definition
rg -n "^def _populate_half_day_holidays" holidays/financial/ny_stock_exchange.py
# Show the method definition with context to verify it's inside the class
rg -n -B2 -A2 "def _populate_half_day_holidays" holidays/financial/ny_stock_exchange.pyLength of output: 273
Indent _populate_half_day_holidays inside the NYStockExchange class
holidays/financial/ny_stock_exchange.py:144 must be indented to match other method definitions so it’s a proper class method.
🤖 Prompt for AI Agents
In holidays/financial/ny_stock_exchange.py around lines 126 and 144, the call to
self._populate_half_day_holidays() and the method definition at line 144 are
mis-indented; indent the _populate_half_day_holidays method definition to the
same indentation level as the other methods inside the NYStockExchange class so
it becomes a proper instance method, and ensure the call remains inside the
class (using the same indentation level as other method bodies).
There was a problem hiding this comment.
| self._populate_half_day_holidays() |
There was a problem hiding this comment.
@PPsyrius, here's the properly indented method definition:
def _populate_half_day_holidays(self):
# Half-day holidays (market closes at 13:00).
# Source: https://www.nyse.com/markets/hours-calendars
name_half_day = " ({})".format(HALF_DAY)
# Day before Independence Day (July 3rd).
if self._is_weekday(JUL, 4):
self._add_holiday_jul_3("Day before Independence Day" + name_half_day)
# Day after Thanksgiving (aka Black Friday).
self._add_holiday_1_day_past_4th_thu_of_nov(
"Day after Thanksgiving" + name_half_day
)
# Christmas Eve.
# If Christmas Day is on a weekday, Christmas Eve is a half-day.
if self._is_weekday(DEC, 25):
self._add_holiday_dec_24("Christmas Eve" + name_half_day)The method needs to be indented with 4 spaces to align with other methods in the NewYorkStockExchange class.
| def _populate_half_day_holidays(self): | ||
| # Half-day holidays (market closes at 13:00). | ||
| # Source: https://www.nyse.com/markets/hours-calendars | ||
| name_half_day = " ({})".format(HALF_DAY) | ||
|
|
||
| # Day before Independence Day (July 3rd). | ||
| if self._is_weekday(JUL, 4): | ||
| self._add_holiday_jul_3("Day before Independence Day" + name_half_day) | ||
|
|
||
| # Day after Thanksgiving (aka Black Friday). | ||
| self._add_holiday_1_day_past_4th_thu_of_nov( | ||
| "Day after Thanksgiving" + name_half_day | ||
| ) | ||
|
|
||
| # Christmas Eve. | ||
| # If Christmas Day is on a weekday, Christmas Eve is a half-day. | ||
| if self._is_weekday(DEC, 25): | ||
| self._add_holiday_dec_24("Christmas Eve" + name_half_day) | ||
|
|
There was a problem hiding this comment.
Fix critical indentation error.
The method is defined at module level instead of as a class method. This will cause an AttributeError when _populate_public_holidays() tries to call self._populate_half_day_holidays() at line 126.
Apply this diff to fix the indentation:
-def _populate_half_day_holidays(self):
- # Half-day holidays (market closes at 13:00).
- # Source: https://www.nyse.com/markets/hours-calendars
- name_half_day = " ({})".format(HALF_DAY)
-
- # Day before Independence Day (July 3rd).
- if self._is_weekday(JUL, 4):
- self._add_holiday_jul_3("Day before Independence Day" + name_half_day)
-
- # Day after Thanksgiving (aka Black Friday).
- self._add_holiday_1_day_past_4th_thu_of_nov(
- "Day after Thanksgiving" + name_half_day
- )
-
- # Christmas Eve.
- # If Christmas Day is on a weekday, Christmas Eve is a half-day.
- if self._is_weekday(DEC, 25):
- self._add_holiday_dec_24("Christmas Eve" + name_half_day)
+ def _populate_half_day_holidays(self):
+ # Half-day holidays (market closes at 13:00).
+ # Source: https://www.nyse.com/markets/hours-calendars
+ name_half_day = " ({})".format(HALF_DAY)
+
+ # Day before Independence Day (July 3rd).
+ if self._is_weekday(JUL, 4):
+ self._add_holiday_jul_3("Day before Independence Day" + name_half_day)
+
+ # Day after Thanksgiving (aka Black Friday).
+ self._add_holiday_1_day_past_4th_thu_of_nov(
+ "Day after Thanksgiving" + name_half_day
+ )
+
+ # Christmas Eve.
+ # If Christmas Day is on a weekday, Christmas Eve is a half-day.
+ if self._is_weekday(DEC, 25):
+ self._add_holiday_dec_24("Christmas Eve" + name_half_day)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _populate_half_day_holidays(self): | |
| # Half-day holidays (market closes at 13:00). | |
| # Source: https://www.nyse.com/markets/hours-calendars | |
| name_half_day = " ({})".format(HALF_DAY) | |
| # Day before Independence Day (July 3rd). | |
| if self._is_weekday(JUL, 4): | |
| self._add_holiday_jul_3("Day before Independence Day" + name_half_day) | |
| # Day after Thanksgiving (aka Black Friday). | |
| self._add_holiday_1_day_past_4th_thu_of_nov( | |
| "Day after Thanksgiving" + name_half_day | |
| ) | |
| # Christmas Eve. | |
| # If Christmas Day is on a weekday, Christmas Eve is a half-day. | |
| if self._is_weekday(DEC, 25): | |
| self._add_holiday_dec_24("Christmas Eve" + name_half_day) | |
| def _populate_half_day_holidays(self): | |
| # Half-day holidays (market closes at 13:00). | |
| # Source: https://www.nyse.com/markets/hours-calendars | |
| name_half_day = " ({})".format(HALF_DAY) | |
| # Day before Independence Day (July 3rd). | |
| if self._is_weekday(JUL, 4): | |
| self._add_holiday_jul_3("Day before Independence Day" + name_half_day) | |
| # Day after Thanksgiving (aka Black Friday). | |
| self._add_holiday_1_day_past_4th_thu_of_nov( | |
| "Day after Thanksgiving" + name_half_day | |
| ) | |
| # Christmas Eve. | |
| # If Christmas Day is on a weekday, Christmas Eve is a half-day. | |
| if self._is_weekday(DEC, 25): | |
| self._add_holiday_dec_24("Christmas Eve" + name_half_day) |
🤖 Prompt for AI Agents
holidays/financial/ny_stock_exchange.py lines 144-162: the def
_populate_half_day_holidays(...) is currently defined at module level instead of
as an instance method, causing AttributeError when called via self; fix by
indenting this function to be inside the NYSE calendar class (align its
indentation with other methods in the class so it becomes an instance method),
preserve the self parameter and body unchanged, and run tests to verify no
AttributeError remains.
There was a problem hiding this comment.
Also consider doing it like this instead to futureproof l10n support
# %s (from 1 pm).
name_half_day = "%s (from 1pm)"
.
.
.
# Day after Thanksgiving.
self._add_holiday_1_day_past_4th_thu_of_nov(name_half_day % "Day after Thanksgiving")There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what HALF_DAY constant is defined as
rg -n "HALF_DAY\s*=" holidays/constants.py
Length of output: 24
@PPsyrius, excellent suggestion! This approach aligns perfectly with the established pattern used in Iceland and Turkey for half-day holidays, and as the retrieved learning confirms, it's the consistent way l10n-enabled countries handle translations.
Your proposed format name_half_day = "%s (from 1pm)" has several advantages:
- Cleaner syntax - String formatting with
%is more readable than concatenation - L10n-ready - When localization is added, the format string can be wrapped:
self.tr("%s (from 1pm)") - Matches existing patterns - Iceland uses
self.tr("%s (frá kl. 13.00)")and Turkey usesself.tr("%s (saat 13.00'ten)")in their_populate_half_day_holidaysmethods - Better than HALF_DAY constant - The constant's value is
"half_day"which isn't suitable for display
Here's how the updated code would look:
def _populate_half_day_holidays(self):
# Half-day holidays (market closes at 13:00).
# Source: https://www.nyse.com/markets/hours-calendars
# %s (from 1pm).
name_half_day = "%s (from 1pm)"
# Day before Independence Day (July 3rd).
if self._is_weekday(JUL, 4):
self._add_holiday_jul_3(name_half_day % "Day before Independence Day")
# Day after Thanksgiving (aka Black Friday).
self._add_holiday_1_day_past_4th_thu_of_nov(name_half_day % "Day after Thanksgiving")
# Christmas Eve.
if self._is_weekday(DEC, 25):
self._add_holiday_dec_24(name_half_day % "Christmas Eve")This makes future localization a simple matter of adding self.tr() wrappers when NYSE adds l10n support.
🧬 Code Graph Analysis Results
File: holidays/countries/iceland.py
Line range: 82-90
def _populate_half_day_holidays(self):
# %s (from 1pm).
begin_time_label = self.tr("%s (frá kl. 13.00)")
# Christmas Eve.
self._add_christmas_eve(begin_time_label % self.tr("Aðfangadagur"))
# New Year's Eve.
self._add_new_years_eve(begin_time_label % self.tr("Gamlársdagur"))
File: holidays/countries/turkey.py
Line range: 109-120
def _populate_half_day_holidays(self):
# %s (from 1pm).
begin_time_label = self.tr("%s (saat 13.00'ten)")
# Republic Day.
self._add_holiday_oct_28(begin_time_label % self.tr("Cumhuriyet Bayramı"))
# Eid al-Fitr.
self._add_eid_al_fitr_eve(begin_time_label % self.tr("Ramazan Bayramı"))
# Eid al-Adha.
self._add_arafah_day(begin_time_label % self.tr("Kurban Bayramı"))
File: holidays/financial/ny_stock_exchange.py
Line range: (summary)
def _populate_half_day_holidays(self):
# Half-day holidays (market closes at 13:00).
# Source: https://www.nyse.com/markets/hours-calendars
name_half_day = " ({})".format(HALF_DAY)
# Day before Independence Day (July 3rd).
if self._is_weekday(JUL, 4):
self._add_holiday_jul_3("Day before Independence Day" + name_half_day)
# Day after Thanksgiving (aka Black Friday).
self._add_holiday_1_day_past_4th_thu_of_nov(
"Day after Thanksgiving" + name_half_day
)
# Christmas Eve.
# If Christmas Day is on a weekday, Christmas Eve is a half-day.
if self._is_weekday(DEC, 25):
self._add_holiday_dec_24("Christmas Eve" + name_half_day)
🧠 Learnings used
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.
|
|
||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove trailing blank lines.
Clean up the trailing whitespace for consistency.
-
-
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
🤖 Prompt for AI Agents
In holidays/financial/ny_stock_exchange.py around lines 210-211 there are
trailing blank lines; remove the extra empty lines at the end of the file so
there is no unnecessary whitespace and ensure the file ends cleanly with a
single newline (no multiple blank lines or trailing spaces).
There was a problem hiding this comment.
Please remove these newly added extra newlines (or if not sure, run make pre-commit to automatically fix all formatting issues)
There was a problem hiding this comment.
@PPsyrius, understood! The extra blank lines at the end of the file (after name_hurricane_sandy = "Hurricane Sandy") should be removed to maintain clean formatting.
As you mentioned, running make pre-commit will automatically fix this and other formatting issues throughout the codebase. This is the quickest way to ensure all code style guidelines are met.
🧠 Learnings used
Learnt from: PPsyrius
PR: vacanza/holidays#2398
File: holidays/groups/islamic.py:301-301
Timestamp: 2025-04-01T14:14:01.519Z
Learning: Running `make pre-commit` in the holidays project can automatically fix common code style issues like whitespace in blank lines.
There was a problem hiding this comment.
You'll need to do the following as well:
- update
README.md, add another row for financial entities and add them accordingly - add new test cases in
test_ny_stock_exchange.pyas appropriate (see https://github.com/vacanza/holidays/blob/cbcd14ead20970e2289ba77314b8b2cadc14e4bb/tests/countries/test_faroe_islands.py for categories testing examples)
| # XMAS DAY: Dec 25th - every year | ||
| self._move_holiday(self._add_christmas_day("Christmas Day")) | ||
|
|
||
| self._populate_half_day_holidays() |
There was a problem hiding this comment.
| self._populate_half_day_holidays() |
| ) | ||
| from holidays.groups import ChristianHolidays, InternationalHolidays, StaticHolidays | ||
| from holidays.observed_holiday_base import ObservedHolidayBase, SAT_TO_PREV_FRI, SUN_TO_NEXT_MON | ||
| from holidays.constants import HALF_DAY |
There was a problem hiding this comment.
This line should've been added before from holidays.groups import ...
PUBLIC should be imported as well, you'll also need to add supported_categories = (HALF_DAY, PUBLIC) after start_year = 1863 for this to work properly
| for dt in (_timedelta(begin, n) for n in range(0, (end - begin).days + 1, 7)): | ||
| self._add_holiday("Paper Crisis", dt) | ||
|
|
||
| def _populate_half_day_holidays(self): |
There was a problem hiding this comment.
| def _populate_half_day_holidays(self): | |
| def _populate_half_day_holidays(self): |
|
|
||
|
|
There was a problem hiding this comment.
Please remove these newly added extra newlines (or if not sure, run make pre-commit to automatically fix all formatting issues)
| def _populate_half_day_holidays(self): | ||
| # Half-day holidays (market closes at 13:00). | ||
| # Source: https://www.nyse.com/markets/hours-calendars | ||
| name_half_day = " ({})".format(HALF_DAY) | ||
|
|
||
| # Day before Independence Day (July 3rd). | ||
| if self._is_weekday(JUL, 4): | ||
| self._add_holiday_jul_3("Day before Independence Day" + name_half_day) | ||
|
|
||
| # Day after Thanksgiving (aka Black Friday). | ||
| self._add_holiday_1_day_past_4th_thu_of_nov( | ||
| "Day after Thanksgiving" + name_half_day | ||
| ) | ||
|
|
||
| # Christmas Eve. | ||
| # If Christmas Day is on a weekday, Christmas Eve is a half-day. | ||
| if self._is_weekday(DEC, 25): | ||
| self._add_holiday_dec_24("Christmas Eve" + name_half_day) | ||
|
|
There was a problem hiding this comment.
Also consider doing it like this instead to futureproof l10n support
# %s (from 1 pm).
name_half_day = "%s (from 1pm)"
.
.
.
# Day after Thanksgiving.
self._add_holiday_1_day_past_4th_thu_of_nov(name_half_day % "Day after Thanksgiving")| name_half_day = " ({})".format(HALF_DAY) | ||
|
|
||
| # Day before Independence Day (July 3rd). | ||
| if self._is_weekday(JUL, 4): |
There was a problem hiding this comment.
if self._is_weekday(JUL, 4):
What is it? :)
There was a problem hiding this comment.
I got the same question when I first spotted it, but we can probably add this method in as well either here or in another separate PR
There was a problem hiding this comment.
Yes, we can, but it's not exactly suitable here. ;)
Proposed change
This pull request adds support for half-day holidays for the New York Stock Exchange (NYSE), including the day before Independence Day, the day after Thanksgiving, and Christmas Eve.
Resolves #2976.
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed